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

Modify DAP2 and DAP4 to optionally allow Fillvalue/Variable mismatch #1155

Merged
merged 4 commits into from
Oct 3, 2018

Conversation

DennisHeimbigner
Copy link
Collaborator

@DennisHeimbigner DennisHeimbigner commented Oct 1, 2018

(Added bullet above so that #1151 is automatically closed when this is merged - @WardF)

re: issue #1151

Modify DAP2 and DAP4 code to handle case when _FillValue type is not
same as the parent variable type.

Specifically:

  1. Define a parameter [fillmismatch] to allow this mismatch;
    default is to disallow.
  2. If allowed, forcibly change the type of the _FillValue to match
    the parent variable.
  3. If allowed Convert the values to match new type
  4. Generate a log message
  5. if not allowed, then fail

Implementing this required some changes to ncdap_test/dapcvt.c
Also added test cases.

Minor Unrelated Changes:

  1. There were a number of warnings about e.g.
    assigning a const char* to a char*. Fix these
  2. In nccopy.1, replace .NP with .IP "n"
    (re PR Fix manpage: warning: macro `NP' not defined. #1144)
  3. fix minor error in ncdump/ocprint

re: issue #1151

Modify DAP2 and DAP4 code to handle case when _FillValue type is not
same as the parent variable type.

Specifically:
1. Define a parameter [fillmismatch] to allow this mismatch;
   default is to disallow.
2. If allowed, forcibly change the type of the _FillValue to match
   the parent variable.
3. If allowed Convert the values to match new type
4. Generate a log message
5. if not allowed, then fail

Implementing this required some changes to ncdap_test/dapcvt.c
Also added test cases.

Minor Unrelated Changes:
1. There were a number of warnings about e.g.
   assigning a const char* to a char*. Fix these
2. In nccopy.1, replace .NP with .IP "n"
   (re PR #1144)
3. fix minor error in ncdump/ocprint
@WardF WardF self-assigned this Oct 1, 2018
@WardF WardF added this to the 4.6.2 milestone Oct 1, 2018
@WardF
Copy link
Member

WardF commented Oct 2, 2018

I'm about to merge this, just following up a weird thing I saw on ARM, but it may have been a symptom of a local network issue.

@WardF
Copy link
Member

WardF commented Oct 2, 2018

I'm seeing failures with tst_filter.sh under autotools with this. @DennisHeimbigner do you see a failure?

@WardF
Copy link
Member

WardF commented Oct 2, 2018

Filter tests weren't occurring as part of the travis CI tests. Turned them on and rerunning tests.

@DennisHeimbigner
Copy link
Collaborator Author

I don't normally run with filters enabled. Let me try it.

@DennisHeimbigner
Copy link
Collaborator Author

I am seeing a memory error inside HDF5. It is occurring in the internal hdf5 memory
allocator/verifier.

@DennisHeimbigner
Copy link
Collaborator Author

I ran again using the master branch and it appears to fail for me in the same way,
so this PR may not be the problem. We may have to do a bisect.

@WardF
Copy link
Member

WardF commented Oct 3, 2018 via email

@DennisHeimbigner
Copy link
Collaborator Author

This may be a deeper problem than we thought. I just ran the same test
on the 4.6.1 release tar and it fails also in the same way. Can you verify?

@DennisHeimbigner
Copy link
Collaborator Author

Just ran with netcdf version 4.6.0 and failed same way.
I think the problem is with the hdf5 library and not netcdf.

@DennisHeimbigner
Copy link
Collaborator Author

Ok, it works with HDF5 version 1.8.20, but fails with 1.10.2 and 1.10.3

@WardF
Copy link
Member

WardF commented Oct 3, 2018

Interesting. I have 1.10.1, but it feels like there was either an API change, or perhaps there is a bug or we are misusing the API somehow. I won't complete the bisect, but I will open a new issue for this and merge this. Ah, I see you just opened #1156. I will merge this and then try to catch up on the other pull requests and start work on the upcoming workshop materials.

@WardF WardF merged commit 0381e1a into master Oct 3, 2018
@WardF WardF deleted the fillmismatch.dmh branch October 3, 2018 20:08
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.

ncdump fails to open a particular file
2 participants