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.

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

use the new NetCDF_jll; require julia 1.3 #88

Merged
merged 9 commits into from
Nov 23, 2020

Conversation

visr
Copy link
Contributor

@visr visr commented Jun 1, 2020

I wanted to give the new NetCDF_jll from JuliaPackaging/Yggdrasil#1090 a test run.

Locally all tests pass except the ones updating the compression level to 4, they seem to stay at 9.

Since this relies on Artifacts being available it needs julia 1.3 minimum and therefore a minor version bump, to 0.11.

https://github.com/JuliaBinaryWrappers/NetCDF_jll.jl

visr added a commit to JuliaGeo/NetCDF.jl that referenced this pull request Jun 1, 2020
I wanted to give the new NetCDF_jll from JuliaPackaging/Yggdrasil#1090 a test run.

Since this relies on Artifacts being available it needs julia 1.3 minimum and therefore a minor version bump, to 0.11.

https://github.com/JuliaBinaryWrappers/NetCDF_jll.jl

Also see Alexander-Barth/NCDatasets.jl#88
@visr
Copy link
Contributor Author

visr commented Jun 2, 2020

Hmm on Windows this is fine apart from the compression issue mentioned above, but on Linux and OSX it complains with Warning! ***HDF5 library version mismatched error***. It looks like HDF5_jll is marked as 1.10.5, but in reality it's more of a mix, see also JuliaPackaging/Yggdrasil#262. I'm not sure if it's safe in this case to go the HDF5_DISABLE_VERSION_CHECK route, so perhaps trying to fix the HDF5_jll is the best option.

$ julia --check-bounds=yes --color=yes -e "if VERSION < v\"0.7.0-DEV.5183\"; Pkg.test(\"${JL_PKG}\", coverage=true); else using Pkg; Pkg.test(coverage=true); end"
   Testing NetCDF
 Resolving package versions...
Warning! ***HDF5 library version mismatched error***
The HDF5 header files used to compile this application do not match
the version used by the HDF5 library to which this application is linked.
Data corruption or segmentation faults may occur if the application continues.
This can happen when an application was compiled by one version of HDF5 but
linked with a different version of static or shared HDF5 library.
You should recompile the application or check your shared library related
settings such as 'LD_LIBRARY_PATH'.
You can, at your own risk, disable this warning by setting the environment
variable 'HDF5_DISABLE_VERSION_CHECK' to a value of '1'.
Setting it to 2 or higher will suppress the warning messages totally.
Headers are 1.10.5, library is 1.10.4
	    SUMMARY OF THE HDF5 CONFIGURATION
	    =================================
General Information:
-------------------
                   HDF5 Version: 1.10.4
                  Configured on: Sat Sep  7 00:17:33 UTC 2019
                  Configured by: root@4ba13cb4b49f
                    Host system: x86_64-unknown-linux-gnu
              Uname information: Linux 4ba13cb4b49f 4.4.0-101-generic #124~14.04.1-Ubuntu SMP Fri Nov 10 19:05:36 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
                       Byte sex: little-endian
             Installation point: /usr/local
Compiling Options:
------------------
                     Build Mode: production
              Debugging Symbols: no
                        Asserts: no
                      Profiling: no
             Optimization Level: high
Linking Options:
----------------
                      Libraries: static, shared
  Statically Linked Executables: 
                        LDFLAGS: 
                     H5_LDFLAGS: 
                     AM_LDFLAGS:  -L/usr/local/lib
                Extra libraries: -lrt -lsz -lz -ldl -lm 
                       Archiver: ar
                       AR_FLAGS: cr
                         Ranlib: ranlib
Languages:
----------
                              C: yes
                     C Compiler: /opt/rh/devtoolset-2/root/usr/bin/gcc ( gcc (GCC) 4.8.2 20140120 )
                       CPPFLAGS: -I/usr/local/include 
                    H5_CPPFLAGS: -D_GNU_SOURCE -D_POSIX_C_SOURCE=200112L   -DNDEBUG -UH5_DEBUG_API
                    AM_CPPFLAGS:  -I/usr/local/include
                        C Flags: -Wl,-strip-all
                     H5 C Flags:  -std=c99  -pedantic -Wall -Wextra -Wbad-function-cast -Wc++-compat -Wcast-align -Wcast-qual -Wconversion -Wdeclaration-after-statement -Wdisabled-optimization -Wfloat-equal -Wformat=2 -Winit-self -Winvalid-pch -Wmissing-declarations -Wmissing-include-dirs -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpacked -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wswitch-default -Wswitch-enum -Wundef -Wunused-macros -Wunsafe-loop-optimizations -Wwrite-strings -Wlogical-op -Wlarger-than=2048 -Wvla -Wsync-nand -Wframe-larger-than=16384 -Wpacked-bitfield-compat -Wstrict-overflow=5 -Wjump-misses-init -Wdouble-promotion -Wtrampolines -Wstack-usage=8192 -Wvector-operation-performance  -s -Wno-inline -Wno-aggregate-return -Wno-missing-format-attribute -Wno-missing-noreturn -Wno-suggest-attribute=const -Wno-suggest-attribute=pure -Wno-suggest-attribute=noreturn -Wno-suggest-attribute=format -O3
                     AM C Flags: 
               Shared C Library: yes
               Static C Library: yes
                        Fortran: no
                            C++: no
                           Java: no
Features:
---------
                   Parallel HDF5: no
Parallel Filtered Dataset Writes: no
              Large Parallel I/O: no
              High-level library: yes
                    Threadsafety: no
             Default API mapping: v110
  With deprecated public symbols: yes
          I/O filters (external): deflate(zlib),szip(encoder)
                             MPE: no
                      Direct VFD: no
                         dmalloc: no
  Packages w/ extra debug output: none
                     API tracing: no
            Using memory checker: no
 Memory allocation sanity checks: no
             Metadata trace file: no
          Function stack tracing: no
       Strict file format checks: no
    Optimization instrumentation: no
Bye...
signal (6): Aborted

@visr
Copy link
Contributor Author

visr commented Jun 2, 2020

Ok, after enabling HDF5_DISABLE_VERSION_CHECK in Travis I can confirm that that works, and only the compression errors remain, same as on Windows. So I removed it again.

@Alexander-Barth
Copy link
Owner

Thank you for trying. Yes, I see this issue too:

~/.julia/artifacts/7d43efbb6eaa15647f7532afa55301abc8e178b0/lib  
 strings libhdf5.so.103.0.0 | grep 'HDF5 library version:'
HDF5 library version: 1.10.4
HDF5 library version: %d.%d.%d
HDF5 library version: 1.10.4
gher17:~/.julia/artifacts/7d43efbb6eaa15647f7532afa55301abc8e178b0/lib  
$ cd -
/home/abarth/.julia/artifacts/7d43efbb6eaa15647f7532afa55301abc8e178b0/include
gher17:~/.julia/artifacts/7d43efbb6eaa15647f7532afa55301abc8e178b0/include  
$ grep 1.10.5 *
H5pubconf.h:#define H5_PACKAGE_STRING "HDF5 1.10.5"
H5pubconf.h:#define H5_PACKAGE_VERSION "1.10.5"
H5pubconf.h:#define H5_VERSION "1.10.5"
H5public.h:#define H5_VERS_INFO    "HDF5 library version: 1.10.5"      /* Full version string */

I guess this is the problem:
https://github.com/JuliaPackaging/Yggdrasil/blob/master/H/HDF5/build_tarballs.jl#L111

@visr
Copy link
Contributor Author

visr commented Jun 2, 2020

Hmm so the native ARM build named 1.10.5 is actually 1.10.4?
https://github.com/JuliaPackaging/Yggdrasil/blob/538b9e8df775877bedad1bff1c75193e3e59dd45/H/HDF5/build_tarballs.jl#L25
Not quite sure how to fix this. Perhaps we should just create an Yggdrasil issue?

@simonbyrne
Copy link

Perhaps we should just create an Yggdrasil issue?

Yes, that seems like the best option.

@visr
Copy link
Contributor Author

visr commented Aug 18, 2020

@simonbyrne is it easy to test Clima on this branch with HDF5_DISABLE_VERSION_CHECK=1 set, like b4cbe9f? Would be another good data point for that approach, besides that NetCDF.jl and NCDatases.jl tests pass.

@Alexander-Barth with regard to the approach you sketch out in JuliaPackaging/Yggdrasil#1112 (comment), there is some doubt about the benefits here: JuliaPackaging/Yggdrasil#1496 (comment).

@Alexander-Barth
Copy link
Owner

Alexander-Barth commented Aug 20, 2020

Yes, wine is not a great solution. Here are the options as far as I can see:

  1. Disabling version checks in HDF5
  2. use wine64 to build 64-bit windows binaries and qemu for Linux on non-x84 architectures but what to do with win 32-bit and Mac OS X and FreeBSD?
  3. make true native builds for Windows and Mac OS outside the current Yggdrasil
  4. life with the version mismatch in HDF5 and the missing (corresponding) header files for HDF5 and use Conda for NetCDF :-(

Did I forget one option? I would say I favor a combination of option 2 + 3 and stop extracting binaries from msys2,h5py... Use true native builds for Windows, Mac OS and Linux x84 and qemu for Linux on ARM, aarch64 and PowerPC

@visr
Copy link
Contributor Author

visr commented Aug 20, 2020

Difficult, none seem ideal. I mainly want to get off of Conda.jl and start using Artifacts, which will provide a much better user experience for installation. Yggdrasil supports just distributing prebuilt binaries, like is done now for HDF5. I guess for NetCDF in Yggdrasil we can just follow whatever is done in the HDF5 Yggdrasil build?

I have been doing quite some NetCDF IO locally using this branch without any issues, so perhaps the differences in patch number are ok? That said, looking at JuliaPackaging/Yggdrasil#1496 and HDFGroup/hdf5#2 it's a bit scary. Although we could just specify that the NetCDF_jll is only compatible with a specific minor version of the HDF5_jll (1.10).

@Alexander-Barth
Copy link
Owner

Yggdrasil supports just distributing prebuilt binaries, like is done now for HDF5.

To clarify my point 3. This is also what we can do in the future, but instead of getting the binaries from some several 3rd parties we would get it from "our" HDF5 binary build repository (for Windows and Mac OS). As far as I understand, this already the case for the native build for arm. Speaking of which, does somebody known if there is a public repo for this artefact?

https://github.com/JuliaPackaging/Yggdrasil/releases/download/HDF5-arm-linux-gnueabihf-v1.10.5/hdf5-arm-linux-gnueabihf-v1.10.5.tar.gz

Another use-case to think is that a user might want to use HDF5 and NetCDF files in the same julia session. Therefore we need to use the same HDF5 libaries in NetCDF (as far as I understand dynamic linking).

visr added a commit to JuliaGeo/NetCDF.jl that referenced this pull request Nov 22, 2020
I wanted to give the new NetCDF_jll from JuliaPackaging/Yggdrasil#1090 a test run.

Since this relies on Artifacts being available it needs julia 1.3 minimum and therefore a minor version bump, to 0.11.

https://github.com/JuliaBinaryWrappers/NetCDF_jll.jl

Also see Alexander-Barth/NCDatasets.jl#88
I wanted to give the new NetCDF_jll from  JuliaPackaging/Yggdrasil#1090 a test run.

Locally all tests pass except the ones updating the compression level to 4, they seem to stay at 9.

Since this relies on Artifacts being available it needs julia 1.3 minimum and therefore a minor version bump, to 0.11.

https://github.com/JuliaBinaryWrappers/NetCDF_jll.jl
I assume this will be more efficient, although I haven't done benchmarks.

This should be still done throughout this file.
to see if otherwise CI passes
@coveralls
Copy link

coveralls commented Nov 22, 2020

Coverage Status

Coverage increased (+0.5%) to 93.986% when pulling 3fcc25a on visr:jll into dbc8695 on Alexander-Barth:master.

@visr
Copy link
Contributor Author

visr commented Nov 22, 2020

This is now updated with a new NetCDF_jll build, that uses a new HDF5_jll build by @musm that upgraded things consistently to 1.12, so there are no header mismatch error anymore here.

This gives all green CI, with the note that I marked a test as broken. This is the test at the end:

# check compression
isshuffled,isdeflated,deflate_level = deflate(v)
@test isshuffled == true
@test isdeflated == true
@test deflate_level == 9

# change compression
deflate(v,false,true,4)
isshuffled,isdeflated,deflate_level = deflate(v)
@test isshuffled == false
@test isdeflated == true
@test_broken deflate_level == 4

Instead the deflate_level remains at 9 on the last line. However enabling shuffling did work, so nc_def_var_deflate did do something.

I just looked around and found Unidata/netcdf-c#1713. So it's a known issue in 4.7.4 that will be fixed in the next version. Since calling deflate twice like this with different settings is probably quite rare, I'd say we just mark it broken and reenable it with the next netcdf-c release.

@musm
Copy link

musm commented Nov 22, 2020

Fantastic!

@Alexander-Barth
Copy link
Owner

This is great ! Thanks a lot @visr !

@Alexander-Barth Alexander-Barth merged commit 39276c5 into Alexander-Barth:master Nov 23, 2020
@visr visr deleted the jll branch November 23, 2020 11:29
@visr visr mentioned this pull request Nov 26, 2020
meggart pushed a commit to JuliaGeo/NetCDF.jl that referenced this pull request Nov 30, 2020
* use the new NetCDF_jll; require julia 1.3

I wanted to give the new NetCDF_jll from JuliaPackaging/Yggdrasil#1090 a test run.

Since this relies on Artifacts being available it needs julia 1.3 minimum and therefore a minor version bump, to 0.11.

https://github.com/JuliaBinaryWrappers/NetCDF_jll.jl

Also see Alexander-Barth/NCDatasets.jl#88

* update CI versions

* update docs projects

This adds NetCDF itself as a dependency, just like is done in PkgTemplates. That means Conda is still a dependency for now until we tag a new release.

* add NetCDF_jll to deps

* update Documenter to 0.25

With prettyurls set to false for local builds, such that you don't need to run a local server to click around.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants