PCMDI / cmor

Climate Model Output Rewriter
BSD 3-Clause "New" or "Revised" License
51 stars 33 forks source link

CMOR calls genutil which is not a dependency #206

Closed durack1 closed 7 years ago

durack1 commented 7 years ago

@dnadeau4 I just ran CMOR3.2.5 which seemed to call genutil (or maybe it was cdms that called genutil issuing the following error:

bash-3.2$ source activate cmor325
(cmor325) bash-3.2$ python runCmorDemo.py 
/Users/durack1/anaconda2/envs/cmor325/lib/python2.7/site-packages/cdms2/axis.py:769: UserWarning: genutil module not present, was not able to determine if axis is level based on units
  "genutil module not present, was not able to determine if axis is level based on units")
Traceback (most recent call last):
  File "runCmorDemo.py", line 22, in <module>
    time.axis = 'T'
AttributeError: 'NoneType' object has no attribute 'axis'
(cmor325) bash-3.2$ conda install genutil -c conda-forge
Fetching package metadata ...........
Solving package specifications: .

Package plan for installation in environment /Users/durack1/anaconda2/envs/cmor325:
The following NEW packages will be INSTALLED:
    genutil: 2.10-np112py27_0 conda-forge
Proceed ([y]/n)? y
genutil-2.10-n 100% |#####################################################################################################################################| Time: 0:00:00 479.95 kB/s

I solved my own problem by installing genutil into this conda environment, but I think it should be added to the conda recipe as a dependency so it's automatically installed for a user

doutriaux1 commented 7 years ago

@durack1 where is runCmorDemo.py

durack1 commented 7 years ago

@doutriaux1 apologies for the delay: https://github.com/PCMDI/obs4MIPs-cmor-tables/tree/master/demo

The demo.zip archive should provide you with the complete test bed, as long as you have a cmor conda env locally installed

doutriaux1 commented 7 years ago

@durack1 CMOR is not calling genutil:

(py3) doutriaux1@loki:[cmor]:[master]:[10313]> grep -R genutil *
Test/nc2asc.py:import genutil
Test/nc2asc.py:# print genutil.minmax(s)
doutriaux1 commented 7 years ago

now is runCMORDemo.py needs it, it's an OBS4MIP issue not a CMOR issue.

durack1 commented 7 years ago

@doutriaux1, I believe the issue is that the cdms that is bundled with the CMOR conda package uses genutil - it is a problem that needs to be resolved

doutriaux1 commented 7 years ago

cdms can take advantage of genutil if present, it is using the udunits bit to detect level based on units. But it's a circular dependency because genutil definitely needs cdms.

doutriaux1 commented 7 years ago

Again if your script needs this aspect of cdms then it should add genutil as a dependency.

durack1 commented 7 years ago

@doutriaux1 the issue in my head is that the CMOR package provides incomplete packages, and that's really not good, genutil is also small, so would be good to add in as a dependency a user (me) doesn't hit the same problem that I have

doutriaux1 commented 7 years ago

@durack1 but (at least before prepare.py) cdms was completely optional, remember CMOR can be used as pure C or pure Fortran. So CMOR itself definitely does not need cdms/genutil. But I will let @dnadeau4 decide if he wants to add cdms and genutil as dependencies for CMOR. Personally I wouldn't.

durack1 commented 7 years ago

@doutriaux1 this is what you get with the standard conda package:

[duro:~] durack1% conda create -n cmor325a cmor -c conda-forge -c pcmdi -c uvcdat
Fetching package metadata ...............
Solving package specifications: .

Package plan for installation in environment /Users/duro/anaconda2/envs/cmor325a:

The following NEW packages will be INSTALLED:

    asn1crypto:      0.22.0-py27_0     conda-forge
    ca-certificates: 2017.4.17-0       conda-forge
    cdat_info:       2.10-py27_3       conda-forge
    cdms2:           2.10-np112py27_3  conda-forge
    cdtime:          2.10-np112py27_0  conda-forge
    certifi:         2017.4.17-py27_0  conda-forge
    cffi:            1.10.0-py27_0     conda-forge
    chardet:         3.0.2-py27_1      conda-forge
    clapack:         3.2.1-0           conda-forge
    cmor:            3.2.5-np112py27_0 pcmdi      
    cryptography:    1.9-py27_0        conda-forge
    curl:            7.52.1-0          conda-forge
    distarray:       2.10-py27_0       conda-forge
    enum34:          1.1.6-py27_1      conda-forge
    esmf:            7.0.0-6           conda-forge
    esmpy:           7.0.0-py27_1      conda-forge
    expat:           2.2.1-0           conda-forge
    g2clib:          1.6.0-3           conda-forge
    hdf4:            4.2.12-0          conda-forge
    hdf5:            1.8.17-11         conda-forge
    idna:            2.5-py27_0        conda-forge
    ipaddress:       1.0.18-py27_0     conda-forge
    jasper:          1.900.1-4         conda-forge
    jpeg:            9b-0              conda-forge
    lapack:          3.6.1-1           conda-forge
    libcdms:         2.10-2            conda-forge
    libcf:           1.0-py27_0        conda-forge
    libdrs:          2.10-1            conda-forge
    libdrs_f:        2.10-py27_0       conda-forge
    libffi:          3.2.1-3           conda-forge
    libgcc:          4.8.5-1                      
    libgfortran:     3.0.0-0           conda-forge
    libnetcdf:       4.4.1.1-4         conda-forge
    libpng:          1.6.28-0          conda-forge
    libtiff:         4.0.6-7           conda-forge
    mkl:             2017.0.3-0                   
    mpich:           3.2-4             conda-forge
    ncurses:         5.9-10            conda-forge
    netcdf-fortran:  4.4.4-3           conda-forge
    numpy:           1.12.1-py27_0                
    openssl:         1.0.2l-0          conda-forge
    ossuuid:         1.6.2-0           conda-forge
    pip:             9.0.1-py27_0      conda-forge
    pycparser:       2.18-py27_0       conda-forge
    pyopenssl:       16.2.0-py27_0     conda-forge
    pysocks:         1.6.7-py27_0      conda-forge
    python:          2.7.13-1          conda-forge
    readline:        6.2-0             conda-forge
    requests:        2.18.1-py27_0     conda-forge
    setuptools:      33.1.1-py27_0     conda-forge
    six:             1.10.0-py27_1     conda-forge
    sqlite:          3.13.0-1          conda-forge
    tk:              8.5.19-1          conda-forge
    udunits2:        2.2.23-0          conda-forge
    urllib3:         1.21.1-py27_1     conda-forge
    wheel:           0.29.0-py27_0     conda-forge
    xz:              5.2.2-0           conda-forge
    zlib:            1.2.11-0          conda-forge

Proceed ([y]/n)? n

Exiting
dnadeau4 commented 7 years ago

@durack1 genutil is not part of CMOR. @taylor13 was not sure if we should add CDMS2. We had not choice with PrePARE.

durack1 commented 7 years ago

@dnadeau4 thanks, the issue is that the cdms2 that IS PART OF CMOR expects genutil to also be installed, so if you're installing cdms2 I suggest adding genutil as a dependency, as cdms2 expects it

doutriaux1 commented 7 years ago

at this point this is turning into a nightmare. I would advise splitting prepare to its own repo.

dnadeau4 commented 7 years ago

Is CDMS2 depending on genutil?

doutriaux1 commented 7 years ago

it's somewhat circular. It does NOT need genutil BUT takes advantage of it (udunits) if present to detect level axes based on units.

doutriaux1 commented 7 years ago

genutil DOES need cdms.

dnadeau4 commented 7 years ago

@durack1, You say that CMOR expects genutil to be installed. If it is true, I will disable that, it should not be needed. Can you clarify?

doutriaux1 commented 7 years ago

@durack1 I guess the question is: "is your script actually failing?" Or do you just see the warning and you decided to raise hell just because?

durack1 commented 7 years ago

@dnadeau4 @doutriaux1 if you're happy with the error:

(cmor325) bash-3.2$ python runCmorDemo.py 
/Users/durack1/anaconda2/envs/cmor325/lib/python2.7/site-packages/cdms2/axis.py:769: UserWarning: genutil module not present, was not able to determine if axis is level based on units
  "genutil module not present, was not able to determine if axis is level based on units")

propagating for anyone using CMOR then I suppose close this issue

I think a complete package with all the required (and optional) dependencies should be the aim for conda package creation, so that every single user that is using the python CMOR version doesn't hit the same problem as I did.. As it's trivial to add genutil to the conda recipe, and as the list of dependencies is already long (see above) I really don't know why we have wasted 30 minutes discussing this..

doutriaux1 commented 7 years ago

@durack1 it is NOT an error but a warning. Now apparently your input data does not define the CF recommened "T" attribute on the axis and it's name is not "time" so cdms was falling back and back and back on other ways to figure the time axis. I agree I don't see why we're wasting time on this.

doutriaux1 commented 7 years ago

Also I still think prepare has no business in CMOR, it's got a life of its own not related to CMOR really. It still recommend moving it to its own repo where we can make @durack1 happy and add more dependencies. @taylor13 @dnadeau4 what do you think?

durack1 commented 7 years ago

@dnadeau4 @doutriaux1 as things are currently working I don't think separating repos at this late stage are a great idea, considering all the time pressures.. And just to note, in conda currently CMOR pulls down 58 packages (see example above), 1 more dependency isn't going to break any storage limits

doutriaux1 commented 7 years ago

@durack1 actually we could go a cleaner and better road, rather than distributing a ziop file you could actually make your zip a package and conda install this. That would be the proper way to distribute it.

durack1 commented 7 years ago

@doutriaux1 @dnadeau4 I would consider myself an advanced user that can solve my own problems (well most of them). But for many other new users of CMOR when they get an error OR a warning, they will complain to the CMOR list, and this will ultimately create more work responding to user queries.. I really think adding a 147.5kB dependency will solve potential queries down the line

Mind you I say all this without a complete understanding of the complexities of how genutil works, and @dnadeau4 has explained to me that there is a complexity with how UDUNITS is called, and that makes what appears to be a simple request considerably more complex

durack1 commented 7 years ago

@doutriaux1 is the comment above in the wrong thread - should it be in PCMDI/obs4MIPs-cmor-tables#54 or PCMDI/obs4MIPs-cmor-tables#56?