Repository navigation
Multidata fields/pseudo-levels in LFRic2UM - #266
Jenny Hickson (jennyhickson) merged 47 commits into
Conversation
|
Currently the problem is that when handling pseudo-levels, it assumes that every level is its own 2d field rather than putting them all in the same field. How to get it to not do this is yet to be discovered |
…and a bit of titivating
Mike Hobson (mike-hobson)
left a comment
There was a problem hiding this comment.
The lfric2UM code is just horrible. You're additions certainly don't make it any worse.
However, I think we could take the opportunity to improve the little bit we are changing here, so I've made some suggestions.
| call log_event(log_scratch_space, LOG_LEVEL_DEBUG) | ||
|
|
||
| ! set level heights to 0.0 | ||
| ! set level heights to 0.0 - potentially except bulev & bhulev? |
There was a problem hiding this comment.
What does "potentially except bulev & bhulev?" mean. The appear to be set to 0.0 like all the other variables.
There was a problem hiding this comment.
In the source UM file, bulev and bhlev are set to the missing data indicator rather than 0.0 - I'm not sure if this is true for all UM files or not, hence the question mark. All the preset variable changes in this file have been separated out to #838 - I think this comment got left behind in the shift
|
|
||
| ! LFRic modules | ||
| use log_mod, only: log_event, LOG_LEVEL_INFO, LOG_LEVEL_ERROR | ||
| use log_mod, only: log_event, log_scratch_space, LOG_LEVEL_INFO, LOG_LEVEL_ERROR |
There was a problem hiding this comment.
I don't think log_scratch_space is used anywhere in this routine - so it can be removed.
There was a problem hiding this comment.
Good point - removed. Also added a log message to the new section as there wasn't one
| if ( get_stashmaster_item(stashcode, pseudt) /= 0 ) then | ||
|
|
||
| if ( get_stashmaster_item(stashcode, pseudl) == 11 ) then | ||
| snow_level_number = ( (level_number + 2)/3 ) * 1000 & | ||
| + (level_number - ((level_number + 2)/3 - 1)*3) | ||
| lookup_int(lbuser5) = snow_level_number | ||
| write(log_scratch_space, '(A,I0)') & | ||
| "Pseudo-level number set as ", snow_level_number | ||
| call log_event(log_scratch_space, LOG_LEVEL_INFO) | ||
|
|
||
| else | ||
| lookup_int(lbuser5) = level_number | ||
| write(log_scratch_space, '(A,I0)') & | ||
| "Pseudo-level number set as ", level_number | ||
| call log_event(log_scratch_space, LOG_LEVEL_INFO) | ||
| end if | ||
|
|
||
| end if |
There was a problem hiding this comment.
I think this code needs a few comments. It would be useful to know what pseudt and pseudl are - and the calculation of snow_level_number just appears to be magic.
There was a problem hiding this comment.
pseudt and pseudl are both described in UM docs C04 as the rest of the "magic words" (bulev, lbuser5, etc) are described in UM docs F03. I think actually they should both be pseudt here - pseudt is the pseudo-level type and is 0 when the field is not a pseudolevel, whereas pseudl is the last pseudo-level code and is subtly different in ways I'm not sure I quite understand (but snow fields are 11 in both, which is the important bit here).
Calculation is pretty much magic - hopefully somewhat better explained by comment? I'm not sure I fully understand it (and have also just realised that this assumes that num_snow_layers is 3 which is true as far as I'm aware but probably best to not have hard coded!)
|
So I've realised that the snow layers read-in/calculation isn't quite correct, as some files can have 11 surface_tiles but still 27 snow_layers_and_tiles. Working on fixing |
Not actually as big a problem as previously thought. Workaround to be used in #564, proper implementation to be done as and when we need 11 surface tiles in the UM output (tiles 10 and 11 are sea and sea-ice and are only used in LFRic) |
Mike Hobson (mike-hobson)
left a comment
There was a problem hiding this comment.
I think these changes address the comments I made in my review. The code comments are now sufficient. The comments you added to the PR are actually a little more detailed and some of that detail could have gone in the code comments, but they are fine.
I'm happy for this to pass sci/tech review. Now it's on to Jenny Hickson (@jennyhickson) for code review...
Jenny Hickson (jennyhickson)
left a comment
There was a problem hiding this comment.
Looks sensible enough and conversations about the lovely lfric2um code and the magic formulas have been covered already.
PR Summary
I recommend that until #806 is on trunk, anyone reviewing the code changes in this PR uses this page instead, as it ignores all changes included in the blocking PR.Blocking PR merged.This PR adds LFRic2UM functionality to regrid LFRic multidata fields to UM pseudolevel fields
Sci/Tech Reviewer: Mike Hobson (@mike-hobson)
Code Reviewer: Jenny Hickson (@jennyhickson)
Code Quality Checklist
Testing
The ex1a lfric2um nwp jobs will fail until the new file from #806 is added to $BIG_DATA_DIRTest Suite Results - lfric_apps - t265_lfric2um_pseudL/run41
Suite Information
Task Information
❌ failed tasks - 4
⌛ waiting tasks - 2
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review