THM is mandatory WRF temp variable #733

braczka · 2024-09-09T20:56:07Z

Description:

Adding documentation in WRF and WRF-DART tutorial to make THM mandatory.

Fixes issue

#728

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update

Documentation changes needed?

My change requires a change to the documentation.
- I have updated the documentation accordingly.

Checklist for merging

Updated changelog entry
Documentation updated
Update conf.py

Checklist for release

Merge into main
Create release from the main branch with appropriate tag
Delete feature-branch

braczka · 2024-09-09T21:02:16Z

Besides documentation, perhaps I could have also forced the code to throw an error if temperature variable T is used as the TYPE_T in wrf_state_variables?

hkershaw-brown

Besides documentation, perhaps I could have also forced the code to throw an error if temperature variable T is used as the TYPE_T in wrf_state_variables?

Yup, it THM is required the model_mod should check
THM is in the state and type_t

hkershaw-brown

see #733 (review)

braczka · 2024-09-13T14:48:07Z

see #733 (review)

Yup, I am on it

braczka · 2024-10-03T22:19:14Z

Now errors out if using T WRF variable with type_t. Should both prevent boundscheck error, and also an end-user unexpectedly using a diagnostic temperature variable instead of prognostic temperature variable (THM), which will deprecate forecast skill.

models/wrf/readme.rst

hkershaw-brown · 2024-10-04T13:23:41Z

Hi Brett, can you share a wrf restart file to test this code with?
Thanks,
Helen

hkershaw-brown · 2024-10-04T13:25:37Z

models/wrf/model_mod.f90

+   ! prevent boundscheck errors. For WRFv4 and later version variable 'T' is 
+   ! diagnostic, thus updating the 'THM' variable (prognostic) is preferred.
+
+   if ( wrf%dom(id)%type_t >= 0 .and. get_type_ind_from_type_string(id,'T') >=0) then


so at the comment, this check isn't checking that THM is in state and type_t

so at the comment, this check isn't checking that THM is in state and type_t

I went back and forth on this -- yes, the code will error out if user includes 'T' as temperature variable. It does not error out if 'THM' is in the DART state, but does give that information in the error message.

I could expand the error by saying:

if ( wrf%dom(id)%type_t >= 0 .and. (get_type_ind_from_type_string(id,'T') >=0 .or. get_type_ind_from_type_string(id,'THM') <=0)) then

Is that preferable?

Suggested edit will now error out if THM is not in DART state but type_t WRF variable is declared.

line 702:
if ( wrf%dom(id)%type_t >= 0 .and. get_type_ind_from_type_string(id,'T') >=0) then

wrf%dom(id)%type_t isn't defined at this stage (unless I'm missing something) so this check wrf%dom(id)%type_t >= 0 is using an uninitialized value for type_t

line 713 is where wrf%dom(id)%type_t is set.
wrf%dom(id)%type_t = get_type_ind_from_type_string(id,'THM')

so to back up a bit, what has to be true to run wrf-dart?

THM has to be in the state and type_t has to be THM?

What's not allowed?
T can not be in the state vector?
T can be in the state vector but can not be type_t?

so to back up a bit, what has to be true to run wrf-dart?

THM has to be in the state and type_t has to be THM?

What's not allowed? T can not be in the state vector? T can be in the state vector but can not be type_t?

The only thing that is not allowed is having the WRF variable 'T' in the DART state because it can throw the 'boundscheck' error which gives a non-intuitive error as described in #728 . There are other secondary reasons to not include 'T' in WRF -- because its diagnostic in WRFv4+, not prognostic, so updating it will not improve the forecast. Users who are used to older WRF versions may not be aware of this.

I also believe that including a 'type_t' WRF variable in the DART state is mandatory because of the way the WRF model_mod.f90 works. Even if you don't intend the 'type_t' variable to be updated by DART, the code still does boundchecks on the type_t variable for many (all?) WRF-DART simulations.

Given the 'type_t' requirement, and secondly, that using variable 'T' is not permitted, this makes including 'THM' mandatory because it is the only replacement variable that I know of for 'T' (3D temperature).

braczka · 2024-10-07T15:41:49Z

Hi Brett, can you share a wrf restart file to test this code with? Thanks, Helen

Yes -- this wrf file has both T and THM to test out functionality:

/glade/work/bmraczka/DART/models/wrf/Akeem_bug/wrfinput_d01.original

braczka · 2024-10-09T14:45:26Z

934d82e Removes check of 'type_t' for error call. Only need to check for WRF variable 'T'

hkershaw-brown · 2024-10-10T15:55:02Z

The code is not enforcing "mandatory THM".
If you run model_mod_check with THM not in the state, you get a boundCheck error:

***************** RUNNING    TEST 4    ***********************
 -- Testing model_interpolate with a single location
**************************************************************
--------------------------------------------------------------
Interpolating QTY_10M_U_WIND_COMPONENT
 --  at "loc_of_interest" location
--------------------------------------------------------------

-------------------------------------------------------------
interpolating at  Lon/Lat(deg): 286.00000000  17.00000000  Vert:    0.0100000 km
interpolating for "QTY_10M_U_WIND_COMPONENT"
-------------------------------------------------------------

forrtl: severe (408): fort: (3): Subscript #2 of the array VAR_SIZE has value -1 which is less than the lower bound of 1

Image              PC                Routine            Line        Source             
model_mod_check    00000000006E75BE  model_mod_mp_boun        6710  model_mod.f90
model_mod_check    000000000055C992  model_mod_mp_mode        1612  model_mod.f90
model_mod_check    00000000007AC643  model_check_utili          86  model_check_utilities_mod.f90
model_mod_check    00000000008E7050  test_interpolate_         339  test_interpolate_threed_sphere.f90
model_mod_check    0000000000800BEA  MAIN__                    257  model_mod_check.f90
model_mod_check    000000000040EE3D  Unknown               Unknown  Unknown
libc-2.31.so       00007F073B6B029D  __libc_start_main     Unknown  Unknown
model_mod_check    000000000040ED6A  Unknown               Unknown  Unknown
hkershaw@derecho4:/glade/derecho/scratch/hkershaw/DART/Bugs/wrf_type_t/runs$

braczka · 2024-10-10T21:46:58Z

The code is not enforcing "mandatory THM". If you run model_mod_check with THM not in the state, you get a boundCheck error:

***************** RUNNING    TEST 4    ***********************
 -- Testing model_interpolate with a single location
**************************************************************
--------------------------------------------------------------
Interpolating QTY_10M_U_WIND_COMPONENT
 --  at "loc_of_interest" location
--------------------------------------------------------------

-------------------------------------------------------------
interpolating at  Lon/Lat(deg): 286.00000000  17.00000000  Vert:    0.0100000 km
interpolating for "QTY_10M_U_WIND_COMPONENT"
-------------------------------------------------------------

forrtl: severe (408): fort: (3): Subscript #2 of the array VAR_SIZE has value -1 which is less than the lower bound of 1

Image              PC                Routine            Line        Source             
model_mod_check    00000000006E75BE  model_mod_mp_boun        6710  model_mod.f90
model_mod_check    000000000055C992  model_mod_mp_mode        1612  model_mod.f90
model_mod_check    00000000007AC643  model_check_utili          86  model_check_utilities_mod.f90
model_mod_check    00000000008E7050  test_interpolate_         339  test_interpolate_threed_sphere.f90
model_mod_check    0000000000800BEA  MAIN__                    257  model_mod_check.f90
model_mod_check    000000000040EE3D  Unknown               Unknown  Unknown
libc-2.31.so       00007F073B6B029D  __libc_start_main     Unknown  Unknown
model_mod_check    000000000040ED6A  Unknown               Unknown  Unknown
hkershaw@derecho4:/glade/derecho/scratch/hkershaw/DART/Bugs/wrf_type_t/runs$

Yep -- trying again with 2d99f18

models/wrf/tutorial/README.rst

hkershaw-brown

Hi Brett, I checkout out the changes and confirmed they error out if no THM is present in model_nml, and error out if T is present in model_nml. So the code looks good.

You have a couple of places in the doc, where you say "Also, including T can give boundscheck errors as described in `issue #728." Since this pull request will error out in static_init_model, if T is in the state, then you won't get to the boundscheck errors. I'd suggest removing these, because if someone gets a boundcheck error with this version of the code, it won't be because they have T in the state. If you think it is helpful to keep these notes in, let me know and we can release as is.

Cheers,
Helen

braczka · 2024-10-22T16:32:13Z

Yes -- let me update the docs to remove the boundscheck comments. This new check avoids the boundscheck issue, so we don't want to lead the user in the wrong direction if they actually get a boundscheck error for other reasons.

Co-authored-by: Helen Kershaw <20047007+hkershaw-brown@users.noreply.github.com>

…iable_doc

braczka · 2024-10-22T17:05:22Z

This is good on my end.

THM is mandatory WRF temp variable

7031c9d

braczka added Documentation Improvements or additions to documentation wrf Weather Research & Forecasting Model labels Sep 9, 2024

braczka requested a review from hkershaw-brown September 9, 2024 21:00

braczka marked this pull request as ready for review September 9, 2024 21:00

hkershaw-brown requested changes Sep 10, 2024

View reviewed changes

hkershaw-brown requested changes Sep 13, 2024

View reviewed changes

braczka added 2 commits October 3, 2024 11:23

Merge remote-tracking branch 'upstream/main' into THM_variable_doc

6436c17

Force error for WRF variable T

0f5f416

hkershaw-brown reviewed Oct 4, 2024

View reviewed changes

models/wrf/readme.rst Outdated Show resolved Hide resolved

hkershaw-brown reviewed Oct 4, 2024

View reviewed changes

braczka added 2 commits October 9, 2024 08:38

Simplify THM error call

fac042b

Correct THM error call

934d82e

Enforcing inclusion of THM

2d99f18

hkershaw-brown reviewed Oct 21, 2024

View reviewed changes

models/wrf/tutorial/README.rst Outdated Show resolved Hide resolved

hkershaw-brown requested changes Oct 21, 2024

View reviewed changes

hkershaw-brown added the release! bundle with next release label Oct 21, 2024

braczka and others added 3 commits October 22, 2024 10:57

Apply suggestions from code review

75b25ce

Co-authored-by: Helen Kershaw <20047007+hkershaw-brown@users.noreply.github.com>

Editing boundscheck error comment

95fbd1a

Merge remote-tracking branch 'upstream/THM_variable_doc' into THM_var…

92dac72

…iable_doc

hkershaw-brown approved these changes Oct 22, 2024

View reviewed changes

hkershaw-brown merged commit 5c4c880 into main Oct 22, 2024
4 checks passed

hkershaw-brown deleted the THM_variable_doc branch October 22, 2024 18:29

hkershaw-brown mentioned this pull request Oct 23, 2024

Using 'T' in WRF model_mod gives 'boundscheck' error #728

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

THM is mandatory WRF temp variable #733

THM is mandatory WRF temp variable #733

braczka commented Sep 9, 2024

braczka commented Sep 9, 2024

hkershaw-brown left a comment

hkershaw-brown left a comment

braczka commented Sep 13, 2024

braczka commented Oct 3, 2024

hkershaw-brown commented Oct 4, 2024

hkershaw-brown Oct 4, 2024

braczka Oct 7, 2024

braczka Oct 7, 2024

hkershaw-brown Oct 7, 2024

hkershaw-brown Oct 7, 2024

braczka Oct 7, 2024

braczka commented Oct 7, 2024

braczka commented Oct 9, 2024

hkershaw-brown commented Oct 10, 2024

braczka commented Oct 10, 2024

hkershaw-brown left a comment

braczka commented Oct 22, 2024

braczka commented Oct 22, 2024

THM is mandatory WRF temp variable #733

THM is mandatory WRF temp variable #733

Conversation

braczka commented Sep 9, 2024

Description:

Fixes issue

Types of changes

Documentation changes needed?

Checklist for merging

Checklist for release

braczka commented Sep 9, 2024

hkershaw-brown left a comment

Choose a reason for hiding this comment

hkershaw-brown left a comment

Choose a reason for hiding this comment

braczka commented Sep 13, 2024

braczka commented Oct 3, 2024

hkershaw-brown commented Oct 4, 2024

hkershaw-brown Oct 4, 2024

Choose a reason for hiding this comment

braczka Oct 7, 2024

Choose a reason for hiding this comment

braczka Oct 7, 2024

Choose a reason for hiding this comment

hkershaw-brown Oct 7, 2024

Choose a reason for hiding this comment

hkershaw-brown Oct 7, 2024

Choose a reason for hiding this comment

braczka Oct 7, 2024

Choose a reason for hiding this comment

braczka commented Oct 7, 2024

braczka commented Oct 9, 2024

hkershaw-brown commented Oct 10, 2024

braczka commented Oct 10, 2024

hkershaw-brown left a comment

Choose a reason for hiding this comment

braczka commented Oct 22, 2024

braczka commented Oct 22, 2024