Feature #104 customization - #106
Conversation
|
@jaredalee and @JohnHalleyGotway |
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
JohnHalleyGotway
left a comment
There was a problem hiding this comment.
Julie, I tentatively approve this PR, but recommend you consider the suggestions I made. In particular, the embedded whitespace in the list of thresholds would cause a runtime error.
I did test these instructions on derecho and note the following:
Warning
The module load instructions failed in 2 spots!
- The module load on this page failed:
module load charliecloud apptainer gcc cuda ncarcompilers
I had to run:
module load ncarenv/24.12 charliecloud apptainer gcc cuda ncarcompilers
Since ncarenv/24.12 is no longer the default. Note that this change is needed in AT LEAST 2 spots in the instrutions.
- Unpacking the GEOG data with
tar -xvzf ${WORKING_DIR}/geog_high_res_mandatory.tar.gz -C ${WORKING_DIR}takes an VERY long time (maybe like 30+ minutes!?).
Would be nice if we could just point to a common spot on the filesystem where WPS_GEOG data already exists.
I ran out of time to actually test the customizations. But I was glad to make it through the original run of hurricane matthew on derecho.
…e derecho instructions.
JohnHalleyGotway
left a comment
There was a problem hiding this comment.
I made some additional changes on this branch, and I tested that the customization instructions work on derecho.
However, I do note that the GenVxMask addition does NOT currently work, but only because I-WRF is currently using METplus version 6.0.0. Once we update it to a newer version of METplus, the GenVxMask customization will work.
|
@JohnHalleyGotway Thank you for your changes and updates. I found one typo where there was a duplication of "use" - "Hurricane Matthew use use case", so I removed that. Unfortunately, that got rid of your approval and I don't have permissions on this repo to override and squash and merge. I'll add @jaredalee to this comment, @JohnHalleyGotway since you're on PTO. Maybe Jared can provide a quick approval and/or squash and merge himself. Jared, John recommended that I look over the changes he pushed to the branch to make sure the language is clear and there weren't typos. He had approved and said to merge, but when I fixed a typo, it got rid of his approval. |
|
Thanks Julie. I just merged it.
John
…On Tue, Aug 4, 2026 at 12:41 PM Julie Prestopnik ***@***.***> wrote:
*jprestop* left a comment (NCAR/i-wrf#106)
<#106 (comment)>
@JohnHalleyGotway <https://github.com/JohnHalleyGotway> Thank you for
your changes and updates. I found one typo where there was a duplication of
"use" - "Hurricane Matthew use use case", so I removed that. Unfortunately,
that got rid of your approval and I don't have permissions on this repo to
override and squash and merge. I'll add @jaredalee
<https://github.com/jaredalee> to this comment, @JohnHalleyGotway
<https://github.com/JohnHalleyGotway> since you're on PTO.
Maybe Jared can provide a quick approval and/or squash and merge himself.
Jared, John recommended that I look over the changes he pushed to the
branch to make sure the language is clear and there weren't typos. He had
approved and said to merge, but when I fixed a typo, it got rid of his
approval.
—
Reply to this email directly, view it on GitHub
<#106?email_source=notifications&email_token=AFA4HKFGZGB4DBDDWMW2G3L5II37ZA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJYGM3TQMJZG442M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5183781979>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFA4HKGKN3S4DLZ3I6GELPL5II37ZAVCNFSNUABFKJSXA33TNF2G64TZHM2TSNBRG4ZTCNBWHNEXG43VMU5TKMBQGE3DSNZWHA4KC5QC>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Pull Request Testing
Reviewed the changes in this branch online and compared with the changes requested in the issue.
Confirmed that the Gallery was removed:
The issue notes three customization additions:
Encourage users to update physics_suite = 'CONUS' to 'tropical' to observe changes to physics suite and their impact on output. (Added in the WRF section)
Create a mask to restrict range of focus to a smaller domain via METplus (Added in the METplus section)
Introduce TMP thresholds to create temperature categories of interest (Added in the METplus section)
Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Review the added documentation to ensure it is sufficient.
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
Do these changes include sufficient testing updates? [Yes]
Will this PR result in changes to the test suite? [No]
If yes, describe the new output and/or changes to the existing output:
Please complete this pull request review by [At your convenience].
Pull Request Checklist
Select: Reviewer(s)
Select: Organization level software support Project or Repository level development cycle Project
Select: Milestone as the version that will include these changes