-
Notifications
You must be signed in to change notification settings - Fork 34
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
base: master
Are you sure you want to change the base?
Conversation
…ed from 'ESMF' to 'esmpy' as of v8.4.0)
Hi, thank you for your making the model2roms package! I also sent you an email with more context. |
… module numpy has no attribute float'
…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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this 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!
There was a problem hiding this comment.
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")?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
…hanges in layout of code snippets
…ors; remove vague comments of @05aeaacd
…this solves AttributeError on grdMODEL.esmfgrid in createAtmosFileUV(); copied relevant lines from convert_MODEL2ROMS() and undid 1 line of @97a2028
A remaining question I have: what is the intention of the atmos forcing part?
|
Hi Jeemijn,
Thank you for the contributions - I`ll take a look at your pull request.
You are correct that the interpolation is not done for atmospheric variables as this is done by ROMS itself if the domain of the atmospheric variable is larger than the grid domain. But in some cases the units differ between ERA5 and what ROMS requires so some changes are made.
I actually dont remember why I used U and not U10 for NorESM but you should use U10 if available. If you implement approaches for using model2roms for IPSL and ACCESS please send me a pull request as this would be super useful code for many who do dynamic downscaling.
Thanks!. T
…On Dec 9, 2024 at 06:45 -0800, Jeemijn Scheen ***@***.***>, wrote:
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 U 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 with u10, v10 available? (I am implementing use cases for the IPSL and ACCESS model at the moment.)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
The intention was to create ROMS applicable files for creating atmospheric forcing. even if ROMS can interpolate the grid, the input files require X number of variable in correct units which this module was intended to provide. The module still require a lot of work but if you can create both ocean and atmospheric forcing for one grid for a given time period it makes life easier to setup a new ROMS model.
…On Dec 9, 2024 at 06:45 -0800, Jeemijn Scheen ***@***.***>, wrote:
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 U 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 with u10, v10 available? (I am implementing use cases for the IPSL and ACCESS model at the moment.)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Because ESMpy was renamed from 'ESMF' to 'esmpy' in v8.4.0
which gave an ImportError for 'import ESMF'.
Supports both names of ESMpy.