-
Notifications
You must be signed in to change notification settings - Fork 26
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
Fixed an indexing bug in blending/remap_dwinds that was causing NaNs. #63
Conversation
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.
Please clean the code
5000 continue | ||
|
||
if(any(isnan(Atm_u))) then |
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.
Do we need those error check? Those are for debug, right?
do k=1,npz | ||
do i=is,ie+1 | ||
do k=1,npz ! 1:65 | ||
do i=is,ie+1 ! 1:3951 |
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.
Do we need 1:3951 as comment here? this number is for NA 3km only.
!psd = Atm_ps | ||
|
||
if(any(isnan(ak0))) then |
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.
Do we need those checks?
@delippi This is still a draft PR. Are you ready for this PR to be reviewed and merged? |
Hi @guoqing-noaa, Thank you and @hu5970 for the quick reviews! I have it in draft mode on purpose. My plan is to test these changes just a bit longer just to be sure nothing else pops up. This error was kind of tricky because it wouldn't happen very often--maybe once per day (running blending 4x per day) in the real-time RRFS_A and only for one member (so 1 out of every 120 times it was run). Then the error was never repeatable. I think if I don't see any errors by tomorrow morning or so this should be ready to go. At which time I will switch it from draft mode and we can merge then. |
DESCRIPTION OF CHANGES:
When implementing blending in the real-time RRFS_A parallel, NaNs would cause the system to fail. The NaNs were traced back to the blending and specifically the remap_dwinds code in which there was an indexing error. This error would only occur sometimes and was not reproducible when rerunning the failed jobs. The fix is when array of size (ie, je) are referenced at either i=ie+1 or j=je+1 just use the value at i-1 or j-1.
TESTS CONDUCTED:
Tested on WCOSS2 running real-time RRFS_A (offline) blending tests 4x per day.
ISSUE (optional):
Fixes issues in #62