-
Notifications
You must be signed in to change notification settings - Fork 154
Update WRF lat-lon and cassini map projection integer #855
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
Conversation
WRF lat-lon projection is integer 6, thus matching this with DART lat-lon projection. Moving cassini to integer 106.
Improved commenting in reference to actual supported WRF map projections for WRF-DART research applications
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.
these changes look good to me, including the comments.
Unfortunately, it looks like the WRF map projection integer naming convention has been stable for quite a while....https://github.com/wrf-model/WRF/blame/master/share/output_wrf.F#L945. I imagine most users enabled the most common projections like lambert, mercator or polar and didn't encounter this very often? |
i just looked through some of my older wrfinput_d01 files and they were all lambert conformal (type 1). these would have been conus runs, not idealized cases. |
Which bit of WRF code did we pull into DART / where are we finding the integer values for the projection codes? In the wrf-model/WRF repo (which Brett linked to earlier today), I am finding different values for the projection codes. In this file for example, the projection code for LATLON is 0 again ... |
I think I will use the debugger to run through the geogrid code again. I know that choosing a lat-Lon grid creates a MAP_PROJ of 6. I also confirmed all other projections (lambert, Mercator, polar) give appropriate MAP_PROJ integer. These internal projection integers in the map_utils code and even the projection character names can change from MAP_PROJ- for example cylindrical is synonymous with lat Lon. Let me run through the debugger to check the integer progression to make sure we get this right. |
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.
Hi Brett,
Now that WRF is once again a supported model rather than a sunsetted one, we need to apply a higher level of rigor when making changes to its code.
For issue #811, it would be helpful to clearly describe the bug—how it manifests, its impact on users, and what the expected behavior should be.
Once the bug is well-defined, a few things should be addressed in the fix:
- The projection parameters should not be set in multiple places.
- The code changes should include unit test(s) to verify the projection functionality.
- The wrf model_mod should write the projection information to the DART log file to support debugging and user clarity.
- Documentation about supported projections should be added to the user documentation rather than included as inline notes in the code.
With these points in mind, I’m going to close this pull request for now.
Cheers,
Helen
INTEGER, PUBLIC, PARAMETER :: PROJ_ROTLL = 203 | ||
|
||
end module misc_definitions_module | ||
|
||
|
||
MODULE map_utils | ||
|
||
! IMPORTANT. WRF-DART USERS PLEASE READ. |
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.
Can't really expect users to know to look in this .f90 to read this info.
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 agree these edits could also be placed in the DART documentation for end users. The reason modified comments were put in the code as well is because we inherited conflicting/confusing comments from the WRF code itself and this confused several of us on the DART team (not necessarily end users), so I think the code comments need to be addressed and clarified as well.
!JPH -- change variable name from map_sphere to more specific map_latlon | ||
integer, parameter :: map_latlon = 0, map_lambert = 1, map_polar_stereo = 2, map_mercator = 3 | ||
integer, parameter :: map_cassini = 6, map_cyl = 5 | ||
integer, parameter :: map_lambert = 1, map_polar_stereo = 2, map_mercator = 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.
why are there two places these magic numbers are set?
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 cannot comment on the original code design -- much of this was following the WRF design from many years ago. I can confirm the changes implemented in this PR provide the intended results using linaro debugger:
Incoming wrfinput_d01 file with following global map projection attributes for lambert conformal example:
:MAP_PROJ = 1 ;
:MAP_PROJ_CHAR = "Lambert Conformal" ;
- DART/models/wrf/model_mod.f90/
subroutine read_wrf_file_attributes Line 7149
Wrf%dom(id)%map_proj = 1
Sets MAP_PROJ variable.
- DART/models/Wrf/Model_mod.f90/
subroutine setup_map_projection Line 7431
Wrf%dom(id)%map_proj = 1
Map_latlon = 6
Map_lambert=1
Map_polar_stereo=2
Map_mercator =3
Map_cyl= 5
Map_cassini=106
- ~/wrf/module_map_utils.f90/subroutine map_set Line 403
Proj_code =1
PROJ_LC =1
Enters LC (lambert conformal) checks
- ~/wrf/module_map_utils.f90. many breaks for each projection subroutine 'set_xxx'
enters subroutine for lambert conformal projection 'set_lc(proj)'
The same can be said for a wrf with lat-lon projection:
:MAP_PROJ = 6 ;
:MAP_PROJ_CHAR = "Lat-Lon" ;
Similarly Wrf%dom(id)%map_proj = 6, thus enters 'Lat-Lon' checks instead of 'cassini'.
Description:
The WRF lat-lon projection (integer 6) was mismatched with the DART map projection (cassini). This fix updates the DART projection to match WRF's .
Fixes issue
Fixes #811
Types of changes
Documentation changes needed?
Tests
Ran some quick tests with WRF-DART tutorial
Checklist for merging
Checklist for release
Testing Datasets