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

Clean up the handling of chunking parameters (the -c flag) in nccopy. #1087

Merged
merged 3 commits into from
Jul 31, 2018

Conversation

DennisHeimbigner
Copy link
Collaborator

@DennisHeimbigner DennisHeimbigner commented Jul 27, 2018

This PR primarily addresses Issue #725.
After a long discussion, I implemented the rules at the end of that issue.
They are documented in nccopy.1.

Additionally, I added a new, per-variable, -c flag that allows
for the direct setting of the chunking parameters for a variable.
The form is
-c var:c1,c2,...ck
where var is the name of the variable (possibly a fully qualified name)
and the ci are the chunksizes for that variable. It must be the case
that the rank of the variable is k. If the new form is used as well
as the old form, then the new form overrides the old form for the
specified variable. Note that multiple occurrences of the new form
-c flag may be specified.

Misc. Other fixes

  1. Added -M option to nccopy to specify the minimum
    allowable chunksize.
  2. Removed the unused variables from bigmeta.c
    (Issue Seeing compile warnings in bigmeta.c #1079)
  3. Fixed failure of nc_test4/tst_filter.sh by using the new -M
    flag (Netcdf cmake #1) to allow filter test on a small chunk size.

After a long discussion, I implemented the rules at the end of that issue.
They are documented in nccopy.1.

Additionally, I added a new, per-variable, -c flag that allows
for the direct setting of the chunking parameters for a variable.
The form is
    -c var:c1,c2,...ck
where var is the name of the variable (possibly a fully qualified name)
and the ci are the chunksizes for that variable. It must be the case
that the rank of the variable is k. If the new form is used as well
as the old form, then the new form overrides the old form for the
specified variable. Note that multiple occurrences of the new form
-c flag may be specified.

Misc. Other fixes
1. Added -M <size> option to nccopy to specify the minimum
   allowable chunksize.
2. Removed the unused variables from bigmeta.c
   (Issue #1079)
3. Fixed failure of nc_test4/tst_filter.sh by using the new -M
   flag (#1) to allow filter test on a small chunk size.
@WardF WardF self-assigned this Jul 30, 2018
@WardF WardF modified the milestones: future, 4.7.0 Jul 30, 2018
@WardF
Copy link
Member

WardF commented Jul 30, 2018

Reviewing now then will merge. Thanks @DennisHeimbigner !

@WardF
Copy link
Member

WardF commented Jul 30, 2018

Seeing a failure on Windows in ncdump/tst_nccopy4.sh. Specifically, in the 'test that nccopy -c can chunk and unchunk files' I'm gettin ga 'Variable not found' error. This is different from the error sometimes seen on master currently.

@WardF
Copy link
Member

WardF commented Jul 30, 2018

Specifically, it is failing on nccopy -c / tmp-chunked.nc tmp-unchunked.nc

@WardF
Copy link
Member

WardF commented Jul 30, 2018

Simple fix, changing -c / to -c //, appears to fix the problem. @DennisHeimbigner any issues with this approach?

@DennisHeimbigner
Copy link
Collaborator Author

DennisHeimbigner commented Jul 30, 2018

nccopy -c / tmp-chunked.nc tmp-unchunked.nc
Interesting. It seemed to be ok with my version of visual studio.
If the string that nccopy gets internally is '/' for '//' then ok. If it is getting
'//', then not ok.
Will '/' work? (that is use quotes on the command line).

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.

Seeing compile warnings in bigmeta.c Nccopy ignores chunk spec when input is netcdf-4 contiguous
2 participants