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

Update package name of ESMPy #18

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

jeemijn
Copy link

@jeemijn jeemijn commented Apr 8, 2024

Because ESMpy was renamed from 'ESMF' to 'esmpy' in v8.4.0
which gave an ImportError for 'import ESMF'.
Supports both names of ESMpy.

@jeemijn
Copy link
Author

jeemijn commented Apr 8, 2024

Hi, thank you for your making the model2roms package! I also sent you an email with more context.

jeemijn added 4 commits April 10, 2024 09:58
…this commit solves 'NameError:name x is not defined' for x=atmosForcing & years & getERA5_1DAYfilename; and 'AttributeError: module grd has no attribute grdClass'; now stuck at NameError for x=nor

Choose a reason for hiding this comment

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

Could you explain why we should change from np.float to float?

Copy link
Author

Choose a reason for hiding this comment

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

I had the following error:

$ python runM2R.py
test
INFO:root:[M2R_run] Initialized logging
INFO:root:[M2R_run] Started model2roms
[...]
INFO:root:[M2R_interp2D] ==> regridSrc2Dst from RHO to U, V and RHO points
Traceback (most recent call last):
  File "/export/lv6/user/jscheen/code_other_repos/model2roms/runM2R.py", line 51, in <module>
    run()
  File "/export/lv6/user/jscheen/code_other_repos/model2roms/runM2R.py", line 34, in run
    model2roms.convert_MODEL2ROMS(confM2R)
  File "/export/lv6/user/jscheen/code_other_repos/model2roms/model2roms.py", line 416, in convert_MODEL2ROMS
    IOsubset.find_subset_indices(confM2R.grdMODEL, min_lat=confM2R.subset[0], max_lat=confM2R.subset[1],
  File "/export/lv6/user/jscheen/code_other_repos/model2roms/IOsubset.py", line 43, in find_subset_indices
    res = np.zeros((Turns, 4), dtype=np.float)                                     ^^^^^^^^
  File "/export/lv6/user/jscheen/.conda/envs/model2roms_env/lib/python3.12/site-packages/numpy/__init__.py", line 324, in __getattr__
    raise AttributeError(__former_attrs__[attr])
AttributeError: module 'numpy' has no attribute 'float'.
`np.float` was a deprecated alias for the builtin `float`. To avoid this error in existing code, use `float` by itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, use `np.float64` here.
The aliases was originally deprecated in NumPy 1.20; for more details and guidance see the original release note at:
    https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations.

I followed the advice written there to change np.float into float. I believe the functioning is the same. Whether you get this error, probably depends on the numpy version. I am using numpy version 1.24.3

Copy link

@trondactea trondactea left a comment

Choose a reason for hiding this comment

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

Could you fix the comments I have and update your pull request? Thanks for contributing!

Choose a reason for hiding this comment

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

Perhaps you could ensure that the code change mentioned here works and if so remove the questions and comments concerned with your code change ("# Is 'grd.Grd' correct? used to be 'grd.grdClass'
# unfinished code; 'nor' & 'mytype' not defined")?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for looking at my suggested code changes!
I am trying to get the entire atmos running without errors, but I am not finished yet. I came further than this line now and I just made a new commit (also removing those vague comments). I will continue another time and will let you know here once I succeed (or not).

Choose a reason for hiding this comment

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

Ok sounds good. Let me know when you have updated the code to a version that is clean and runs and I will look at your changes again and merge your code with mine

Copy link
Author

@jeemijn jeemijn Dec 9, 2024

Choose a reason for hiding this comment

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

@trondactea I made an update and it is finally working! See the new commits below. I can now also run with "create_atmos_forcing = True" without errors, even though this does not execute much code in the end and the atmos file is not filled (except for NORESM, which I am not using). Could you take a look at my changes?

…this solves AttributeError on grdMODEL.esmfgrid in createAtmosFileUV(); copied relevant lines from convert_MODEL2ROMS() and undid 1 line of @97a2028
@jeemijn
Copy link
Author

jeemijn commented Dec 9, 2024

A remaining question I have: what is the intention of the atmos forcing part?

  • I realized only recently that ROMS can interpolate the atmospheric variables by itself (unfortunately the ROMS wiki page "atmospheric forcing" is empty) so this explains why you didn't have to make a loop over all atmospheric variables because you don't need to interpolate those at all. Is that correct?
  • I saw that for NorESM you create U,V out of U10 and wind stress. Is this because V10 is not available? Is it correct that this is not needed for other use cases, such as ERA5 or CMIP models that have U10, V10 available? (I am implementing use cases for the IPSL and ACCESS model at the moment.)

@trondactea
Copy link

trondactea commented Dec 9, 2024 via email

@trondactea
Copy link

trondactea commented Dec 9, 2024 via email

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.

2 participants