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

Add bitgroom support to NCZarr #2140

Closed
wants to merge 14 commits into from

Conversation

DennisHeimbigner
Copy link
Collaborator

re: PR #2088

Primary changes:

  • Add NCZarr-specific quantize function to the dispatch table.
  • Copy quantize code from libhdf5
  • Add quantize invocation to zvar.c
  • Add support for _QuantizeBitgroomNumberOfSignificantDigits to ncgen.
  • Copy quantize test from nc_test4 to nczarr_tests. Remove some parts that are not relevant to NCZarr.

Other Changes:

  • Break zsync.c into zsync.c (writing) and zload.c (reading).
  • Clean up the fill value handling (many changes)
  • Disable atexit() under Windows
  • Move ncjson to libdispatch
  • Add documentation of differences between netcdf-4 and NCZarr, especially WRT fill value.
  • Some mingw fixes
  • Remove some cruft
  • Cleanup the handling of scalars

re: PR Unidata#2088

Primary changes:
* Add NCZarr-specific quantize function to the dispatch table.
* Copy quantize code from libhdf5
* Add quantize invocation to zvar.c
* Add support for _QuantizeBitgroomNumberOfSignificantDigits to ncgen.
* Copy quantize test from nc_test4 to nczarr_tests. Remove some parts that are not relevant to NCZarr.

Other Changes:
* Break zsync.c into zsync.c (writing) and zload.c (reading).
* Clean up the fill value handling (many changes)
* Disable atexit() under Windows
* Move ncjson to libdispatch
* Add documentation of differences between netcdf-4 and NCZarr, especially WRT fill value.
* Some mingw fixes
* Remove some cruft
* Cleanup the handling of scalars
@lgtm-com
Copy link

lgtm-com bot commented Nov 3, 2021

This pull request introduces 1 alert when merging 2543a8e into 0e205f9 - view on LGTM.com

new alerts:

  • 1 for Too few arguments to formatting function


/* Create a netcdf-4 file with two vars. Attempt
* quantization. It will work, eventually... */
if (nc_create(file_name, NC_NETCDF4|NC_CLOBBER, &ncid)) ERR;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DennisHeimbigner I don't understand how this tests the nczarr quantize. Seems like you would need to change the mode flags of the nc_create() calls, right? Otherwise these tests are just creating a netCDF/HDF5 file and testing quantize works there...

Copy link
Collaborator Author

@DennisHeimbigner DennisHeimbigner Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file_name is a URL in this case: e.g. s3://bucket/zarrfile#mode=nczarr,s3
Look at the file run_quantize.sh to see how this works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you don't need to use NC_ZARR if the filename is a URL?

Does NC_NETCDF4 have any effect in this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the URL is properly structured, then even NC_NETCDF4 is not needed.

@DennisHeimbigner
Copy link
Collaborator Author

DennisHeimbigner commented Jan 16, 2022

Close to split into multiple PRs.

DennisHeimbigner added a commit to DennisHeimbigner/netcdf-c that referenced this pull request Jan 24, 2022
re: PR Unidata#2088
re: PR Unidata#2130
replaces: Unidata#2140

Changes:
* Add NCZarr-specific quantize functions to the dispatch table.
* Copy (modified) quantize code from libhdf5 to NCZarr
* Add quantize invocation to zvar.c
* Add support for _QuantizeBitgroomNumberOfSignificantDigits
and _QuantizeGranularBitgroomNumberOfSignificantDigits to ncgen.
* Modify nc_test4/tst_quantize.c to allow it to be used both for hdf5
  and for nczarr.
* Make dap4 properly handle quantize functions in dispatch table.
* Add quantize attribute support to ncgen.

Other changes:
* Caught and fixed some S3 problems
* Fixed some nczarr fillvalue problems.
* Fixed some nczarr cache problems.
* Cleanup some flaws in libdispatch/dinfermodel.c
* Allow byterange requests to S3 be readable by dinfermodel.c/check_file_type
* Remove the libnczarr ztracedispatch code (big change).
@DennisHeimbigner DennisHeimbigner deleted the zarrbg.dmh branch January 25, 2022 19:36
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.

2 participants