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

Replace cdms2 with netCDF4 in PrePARE and Python tests #493

Merged
merged 21 commits into from
Jun 19, 2019

Conversation

mauzey1
Copy link
Collaborator

@mauzey1 mauzey1 commented May 24, 2019

Resolves #490

This PR replaces the use of cdms2 in PrePARE and some of the Python tests with netCDF4.

Note: The MacOS CircleCI checks failed due to an error when creating the conda environment.

MD5MismatchError: Conda detected a mismatch between the expected content and downloaded content
for url 'https://repo.anaconda.com/pkgs/main/osx-64/libcxx-4.0.1-h579ed51_0.tar.bz2'.
  download saved to: /Users/distiller/project/workspace/test_macos_cmor/miniconda/pkgs/libcxx-4.0.1-h579ed51_0.tar.bz2
  expected md5 sum: 28471619a3a79da80d9b5747f1ec0221
  actual md5 sum: 291e61684d014641092020bdb1acb096

However, the changes that triggered the failed tests (9f56aa6) didn't change the CircleCI configuration script. The previous commit (8ee7112) that did change the CircleCI and Travis scripts was passing tests.

@mauzey1 mauzey1 changed the title Replace cdms2 with netCDF4 Replace cdms2 with netCDF4 in PrePARE and Python tests May 24, 2019
@doutriaux1
Copy link
Collaborator

@muryanto1 this is the same error as you. I think a package got corrupted on their server

@doutriaux1
Copy link
Collaborator

@mauzey1 can we have some test with cdms and some w/o like that we test both ways

@@ -457,15 +457,15 @@ def ControlVocab(self, ncfile, variable=None, print_all=True):
for attr in ['branch_time_in_child', 'branch_time_in_parent']:
if attr in list(self.dictGbl.keys()):
self.set_double_value(attr)
if not isinstance(self.dictGbl[attr], numpy.float64):
if not numpy.issubdtype(self.dictGbl[attr], numpy.float64):
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice!

@@ -535,7 +535,7 @@ def ControlVocab(self, ncfile, variable=None, print_all=True):
# -------------------------------------------------------------------
varid = cmip6_cv.setup_variable(variable_cmor_entry,
self.dictVar['units'],
self.dictVar['_FillValue'][0],
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this due to the switch from cdms2 to netcdf?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cdms2 was returning values as numpy ndarrays even if they were just one value. netCDF4 returns numpy scalars for single values.

@mauzey1
Copy link
Collaborator Author

mauzey1 commented May 29, 2019

@doutriaux1 Should we include cdms2 along side netCDF4, or just use cdms2 in the testing environment for cdms tests?

@mauzey1
Copy link
Collaborator Author

mauzey1 commented May 29, 2019

@doutriaux1 The tests that I previously modified to use netCDF4 instead of cdms2 now have test cases for both. cdms2 is installed in the testing environment but not for the build.

@mauzey1
Copy link
Collaborator Author

mauzey1 commented Jun 4, 2019

@doutriaux1
The test test_python_max_variables.py is failing on the MacOS checks. It's giving this error:

HDF5-DIAG: Error detected in HDF5 (1.10.4) thread 140735957398464:
  #000: H5F.c line 444 in H5Fcreate(): unable to create file
    major: File accessibilty
    minor: Unable to open file
  #001: H5Fint.c line 1364 in H5F__create(): unable to open file
    major: File accessibilty
    minor: Unable to open file
  #002: H5Fint.c line 1559 in H5F_open(): unable to open file: name = 'CMIP6/CMIP6/ISMIP6/PCMDI/PCMDI-test-1-0/piControl-withism/r3i1p1f1/Amon/ta/gn/v20190531/ta_Amon_PCMDI-test-1-0_piControl-withism_r3i1p1f1_gnaG4OFK13537.nc', tent_flags = 13
    major: File accessibilty
    minor: Unable to open file
  #003: H5FD.c line 734 in H5FD_open(): open failed
    major: Virtual File Layer
    minor: Unable to initialize object
  #004: H5FDsec2.c line 346 in H5FD_sec2_open(): unable to open file: name = 'CMIP6/CMIP6/ISMIP6/PCMDI/PCMDI-test-1-0/piControl-withism/r3i1p1f1/Amon/ta/gn/v20190531/ta_Amon_PCMDI-test-1-0_piControl-withism_r3i1p1f1_gnaG4OFK13537.nc', errno = 24, error message = 'Too many open files', flags = 13, o_flags = 602
    major: File accessibilty
    minor: Unable to open file

I suspect it is due to it creating a lot of open file handles faster than it can close them.

The Python 3.7 MacOS check is getting an error at line 153 in Test/test_python_joerg_3.py.

file1 = cmor.close(ivar1, True)
file2 = cmor.close(ivar2, True)
print('File1:', file1)
print('File2:', file2)
cmor.close()
print(cmor.close(ivar1, True))
cmor.close()

It produces this error:

Traceback (most recent call last):
  File "Test/test_python_joerg_3.py", line 153, in <module>
    print(cmor.close(ivar1, True))
  File "/Users/distiller/project/workspace/test_macos_cmor/miniconda/envs/py3.7/lib/python3.7/site-packages/CMOR-3.4.0-py3.7-macosx-10.9-x86_64.egg/cmor/pywrapper.py", line 944, in close
    return _cmor.close(var_id, 1, 0)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xaf in position 1: invalid start byte
make[1]: *** [test_a_python] Error 1
make: *** [test_python] Error 2
Exited with code 2

It seems that Python 3.7 on OSX is having an issue with CMOR returning a file name when closing a variable that was already closed.

@mauzey1
Copy link
Collaborator Author

mauzey1 commented Jun 4, 2019

@doutriaux1 Looking at the function cmor_close_variable, the parts that set the parameter file_name are skipped if the variable is closed. This will cause _cmor.close in pywrapper.py to return an uninitialized string.

Should we set file_name to an empty string if the variable is closed? Or, should we throw an error or warning if attempting to close an already closed variable?

edit: If preserve is not NULL, then the variable's file name should still exist otherwise it gets set to an empty string. So we should be passing the file name even if the variable is closed.

@mauzey1
Copy link
Collaborator Author

mauzey1 commented Jun 5, 2019

@taylor13 @durack1 What should be the behavior of cmor_close(var_id) if var_id is for an already closed variable, or if CMOR is closed? Warnings or errors? Should it still return a file name for the variable if it was closed with preserve set to true?

@taylor13
Copy link
Collaborator

taylor13 commented Jun 5, 2019

In this case I think a "Warning" should be raised (not an "error"). If "preserve" is set to true, I think is would be nice to return the file name (even if the file had already been closed), but if this is difficult to do, then don't bother. [We should probably document the behavior in the CMOR documentation.

@mauzey1
Copy link
Collaborator Author

mauzey1 commented Jun 6, 2019

I'm not sure about how to handle the test_python_max_variables.py error. It seems to stem from an open file descriptor limit on CircleCI's MacOS. It runs fine on my Macbook. All it seems to be doing is creating, writing, and closing a variable 600 times. I will comment it out of the Makefile for the time being.

@mauzey1
Copy link
Collaborator Author

mauzey1 commented Jun 11, 2019

@doutriaux1 Do you think this pull request can be approved? Should I put the changes for the inclusion of the Python tests in another PR?

@doutriaux1
Copy link
Collaborator

@mauzey1 how does make test knows about which tests need cdms2 and which do not?

@mauzey1
Copy link
Collaborator Author

mauzey1 commented Jun 11, 2019

@doutriaux1 There are some Python tests, such as test_python_history.py, that check if cdms2 is installed. If not installed then the test will exit.

There are similar checks for cdms2 in the CMIP6 CV tests. The code below will skip the test case if cdms2 is not installed.

@unittest.skipIf(pkgutil.find_loader("cdms2") is None, "cdms2 not installed")
def testCMIP6_CDMS2(self):
import cdms2

Since cdms2 is a Python libary, should we only rerun the Python tests after installing cdms2?

@doutriaux1
Copy link
Collaborator

@mauzey1 looks good.

@mauzey1 mauzey1 merged commit b8e0bfc into master Jun 19, 2019
@doutriaux1 doutriaux1 deleted the 490_replace_cdms2_with_netcdf4 branch June 19, 2019 19:50
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.

Builds breaking from cdms2 issue related to libnetcdf
3 participants