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

Indexing fix in multiple python scripts #1287

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

mpaiao
Copy link
Contributor

@mpaiao mpaiao commented Nov 18, 2024

Minor updates on multiple python scripts, for indexing and for using netcdf from scipy.io libraries

Description:

I don't know if this is only happening on my computer, but FatesPFTIndexSwapper.py started to issue the following indexing error when it attempted to copy scalars:

Creating Variable:  fates_canopy_closure_thresh
Traceback (most recent call last):
  File "/path_to/E3SM/components/elm/src/external_models/fates/tools/FatesPFTIndexSwapper.py", line 296, in <module>
    main(sys.argv)
    ~~~~^^^^^^^^^^
  File "/path_to/Models/E3SM/components/elm/src/external_models/fates/tools/FatesPFTIndexSwapper.py", line 207, in main
    out_var.assignValue(float(fp_in.variables.get(key).data))
    ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.13/site-packages/scipy/io/_netcdf.py", line 943, in assignValue
    self.data[:] = value
    ~~~~~~~~~^^^
IndexError: too many indices for array: array is 0-dimensional, but 1 were indexed
<sys>:0: RuntimeWarning: Cannot close a netcdf_file opened with mmap=True, when netcdf_variables or arrays referring to its data still exist. All data arrays obtained from such files refer directly to data on disk, and must be copied before the file can be cleanly closed. (See netcdf_file docstring for more information on mmap.)

The suggested change in the code seems to work fine (though I still get the RuntimeWarning message). I made similar updates in a few other scripts. I also updated ncvarsort.py to use scipy.

Collaborators:

Expectation of Answer Changes:

No change in the FATES code, just the tool.

Checklist

If this is your first time contributing, please read the CONTRIBUTING document.

All checklist items must be checked to enable merging this pull request:

Contributor

  • The in-code documentation has been updated with descriptive comments
  • The documentation has been assessed to determine if updates are necessary

Integrator

  • FATES PASS/FAIL regression tests were run
  • Evaluation of test results for answer changes was performed and results provided

Documentation

Test Results:

CTSM (or) E3SM (specify which) test hash-tag:

CTSM (or) E3SM (specify which) baseline hash-tag:

FATES baseline hash-tag:

Test Output:

…ation, the code

would crash with scalars. The updates should be equivalent to the old code but do not
cause errors.
@glemieux glemieux added the type: tools This PR adds or updates support tools. No regression testing necessary. label Nov 18, 2024
@mpaiao mpaiao changed the title Indexing fix in FatesPFTIndexSwapper.py Indexing fix in multiple python scripts Nov 18, 2024
@glemieux glemieux self-assigned this Nov 18, 2024
@@ -4,11 +4,19 @@
# --input or --fin: input filename.
# --output or --fout: output filename. If missing, will assume its directly modifying the input file, and will prompt unless -O is specified

import netCDF4 as nc
#import netCDF4 as nc
Copy link
Contributor

@rgknox rgknox Nov 22, 2024

Choose a reason for hiding this comment

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

Its nice that we are being more consistent in the library usage, this will lower the dependency threshold, nice catch @mpaiao

varin = dsin.variables.get(v_name)
v_type = dsin.variables[v_name].typecode()
v_dims = varin.dimensions
print(" V_NAME = ",v_name,"; V_TYPE = ",v_type,"; V_DIMS = ",v_dims)
Copy link
Contributor

Choose a reason for hiding this comment

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

do you want to keep this print statement, or was it just for debugging?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: tools This PR adds or updates support tools. No regression testing necessary.
Projects
Development

Successfully merging this pull request may close these issues.

3 participants