-
Notifications
You must be signed in to change notification settings - Fork 10
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
GitHub Issue NOAA-EMC/GSI#604 Undefined values found in radar reflectivity direct DA #605
Merged
ShunLiu-NOAA
merged 1 commit into
NOAA-EMC:develop
from
shoyokota:feature/PR_NOAA-EMC_EnVar-DBZ2
Sep 30, 2023
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@shoyokota I think t4dv is also undefined as shown below. Is this an issue when you ran your case with debug mode gsi? That will be great if it is also taken care of in this PR. From my point of view, it think it could be defined as timeb+ (analysis time - the beginning time of the time window). You could see how it is set in other observation 's setup subroutine

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.
@TingLei-NOAA This t4dv is only used in thinning independently in each timeslot. However, it is assumed that all the inputted reflectivity observations were observed at the analysis time here. So, we don't have to input any specific times as t4dv here, and the result doesn't depend on t4dv as long as it is the same in all reflectivity observations. Here, I modified here to timedif=zero simply to prevent undefined values.
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.
@shoyokota if you runs doesn't go through this part. We could let it be the current codes (with that comment) for future developers to take care . It should be analysis time - the begin of the time window. We don't need to enforce this part also work with that all obs are being at the analysis time . And, even with assumption/requirement that dbz obs being on the analysis time, the fgat/4dvar could still be invoked for other observations could be distributed on different time levels. So, I prefer leaving it to be updated in the future rather than making a change with unnecessary assumptions.
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.
This part is computed in the case of radar_no_thinning=F even now, so I think some kind of fix is needed. Is it better to add comments such as 'observation time (hour) should be input for 4D observations in 3D thinning, but 3D observations here' at the right of 'timedif=zero'?
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.
@shoyokota Thanks for the clarification. I will add my thoughts on those specific codes respectively.