Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix _typed_load fast path for Julia 1.10 (currently nightly) #1075

Merged
merged 4 commits into from
May 30, 2023
Merged

Fix _typed_load fast path for Julia 1.10 (currently nightly) #1075

merged 4 commits into from
May 30, 2023

Conversation

mkitti
Copy link
Member

@mkitti mkitti commented May 30, 2023 •
edited
Loading

Julia master seems to have removed Base._memcpy!. This is to be expected given that this is a private function.

xref: JuliaLang/julia#49550

@mkitti
Copy link
Member Author

mkitti commented May 30, 2023

@t-bltg I believe you are responsible for adding this initially. Can you revisit this?

@mkitti mkitti marked this pull request as ready for review May 30, 2023 02:04
@mkitti
Copy link
Member Author

mkitti commented May 30, 2023

This should fix nightly. I do want to review this piece of code more carefully though.

@mkitti mkitti merged commit 4957fb8 into JuliaIO:master May 30, 2023
@ranocha
Copy link
Contributor

ranocha commented Aug 18, 2023

@mkitti It looks like there is no new release of HDF5.jl including this fix? At least we get

  Got exception outside of a @test
  LoadError: UndefVarError: `_memcpy!` not defined
  Stacktrace:
    [1] _generic_read(::HDF5.Dataset, ::HDF5.Datatype, ::Type{Cstring}, ::Nothing)
      @ HDF5 ~/.julia/packages/HDF5/aiZLs/src/readwrite.jl:197
    [2] generic_read(::HDF5.Dataset, ::HDF5.Datatype, ::Type{Cstring})
      @ HDF5 ~/.julia/packages/HDF5/aiZLs/src/readwrite.jl:134
    [3] read(obj::HDF5.Dataset)
      @ HDF5 ~/.julia/packages/HDF5/aiZLs/src/readwrite.jl:39
    [4] (::Trixi.var"#282#299")(file::HDF5.File)
      @ Trixi ~/work/Trixi.jl/Trixi.jl/src/meshes/mesh_io.jl:308
    [5] (::HDF5.var"#17#18"{HDF5.HDF5Context, @Kwargs{}, Trixi.var"#282#299", HDF5.File})()
      @ HDF5 ~/.julia/packages/HDF5/aiZLs/src/file.jl:98
    [6] task_local_storage(body::HDF5.var"#17#18"{HDF5.HDF5Context, @Kwargs{}, Trixi.var"#282#299", HDF5.File}, key::Symbol, val::HDF5.HDF5Context)
      @ Base ./task.jl:297
    [7] #h5open#16
      @ HDF5 ~/.julia/packages/HDF5/aiZLs/src/file.jl:93 [inlined]
    [8] h5open(filename::String, mode::String)
      @ HDF5 ~/.julia/packages/HDF5/aiZLs/src/file.jl:91 [inlined]
...

in our CI runs testing Julia v1.10.0-beta2, see https://github.com/trixi-framework/Trixi.jl/actions/runs/5898667882/job/16000054066?pr=1562#step:7:2270
It would be great to be able to use HDF5.jl on Julia v1.10 once it's released

@ranocha ranocha mentioned this pull request Aug 18, 2023
Merged
@mkitti
Copy link
Member Author

mkitti commented Aug 18, 2023

Sorry. I'm working on a breaking release. Targeting Monday in ~96 hours.

@ranocha
Copy link
Contributor

ranocha commented Aug 18, 2023

Great, thanks a lot! I'm looking forward to it

@mkitti mkitti mentioned this pull request Aug 20, 2023
Merged
@mkitti
Copy link
Member Author

mkitti commented Aug 20, 2023

I'm planning a 0.16.16 release to fix this sooner than 0.17:
#1098

mkitti added a commit that referenced this pull request Aug 23, 2023
@mkitti @simonbyrne
@musm @stevengj
Port non-breaking changes for 0.16.16 (#1098)
c52831e 
* Fix _typed_load fast path for Julia 1.10 (currently nightly) (#1075)

* Fix _typed_load fast path for Julia 1.10 (currently nightly) by thresholding on `1.10.0-DEV.1390`
* Use `Libc.memcpy` after that threshold

* Add initial support for H5Dchunk_iter (#1031)

* Add initial support for H5Dchunk_iter

* Implement h5d_chunk_iter_helper

* Implement HDF5.get_all_chunk_info

* Make tests pass via HDF5 1.14.0

* Apply formatting

* Test filters with filter_mask via H5Dchunk_iter

* Require functions to return an integer

* Provide index based chunk iteration, rename to HDF5.get_chunk_info_all

* Fix formatting

* Fix documentation

* Fix documentation

* Improve testing

* Always define _get_chunk_info_all_by_iter for documenter

* Update src/datasets.jl

Co-authored-by: Simon Byrne <simonbyrne@gmail.com>

* Precompile get_chunk_info_all implementations before benchmarking

* Fix documentation

* Fix tests

* Formatting

---------

Co-authored-by: Simon Byrne <simonbyrne@gmail.com>

* Simplify formatter check (#1078)

This will display the diff, and return a non-zero exit code if there are changes

* Fix #1083 maxdims -> max_dims in dataspace documentation (#1085)

* Update light and dark logos for readme (#1087)

* Fixup readme logo links and readme style (#1088)

* Fixup readme logo links and readme style

* Tweak

* Tweak logo centering

* Upload curves directly instead of fonts for logo independence (#1089)

* Allow create_dataset to take a Type and Dataspace, Fix #1084 (#1086)

* Fix #1083 maxdims -> max_dims in dataspace documentation

* Fix #1084, create_dataset with Type and Dataspace

Also expand definitions for dataspace with two positional arguments.
Pin Blosc_jll to 1.21.2 if AVX2 is not detected. See Blosc/c-blosc#371

* Formatter

* Attempt to fix documentation (#1091)

* doc typo: attributes(parent), not attribute(parent) (#1095)

* Fix tests for Julia 1.3

* Fix tests for Julia 1.3, Windows SZIP is broken again

* Use static if block to fix 1.3

* Fix version and formatting

* Fix tests

* Bump version to 0.16.16

* Formatting

---------

Co-authored-by: Simon Byrne <simonbyrne@gmail.com>
Co-authored-by: Mustafa M <mus-m@outlook.com>
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mkitti @ranocha