Update the sprint branch again with develop changes mostly for BUFR (via PR1100)#1138
Conversation
|
Making this PR a draft until fixing a bunch of the 92 failed ctests. Do not review yet. |
src/gsi-ncdiag/gsi_ncdiag.py
Outdated
| 'QI_with_FC': ('percentConfidenceWithForecast', 'float'), | ||
| 'QI_without_FC': ('percentConfidenceWithoutForecast', 'float'), | ||
| 'LaunchTime': ('LaunchTime', 'float'), | ||
| 'releaseTime': ('releaseTime', 'integer'), |
There was a problem hiding this comment.
believe the left hand side is the GSI variable name so should remain as LaunchTime but am not entirely sure
There was a problem hiding this comment.
Agreed. Good catch. Fixing in next push of code.
…a-converters-greg3
…a-converters-greg3
|
@gthompsnJCSDA this is quite large, is there anyway to break it up and get a part of it in now |
srherbener
left a comment
srherbener
left a comment
There was a problem hiding this comment.
Great progress - down to 15 test failures on all three compilers. The rest of the tests can be addressed in subsequent PRs. Thanks for all your hard work to get this accompilished!
|
@BenjaminRuston did you mean to reset your review? Given that we will accept with test failures for this PR, is this ready to merge now? The companion PR in ioda is ready. thanks! |
@jeromebarre is this referring to the averaging kernel variables that cannot be 2D yet. That is, they need to remain as a set of 1D variables with the level (?) number attached as a suffix. If so, which specific sets of variables need to remain 1D? Thanks! |
|
@jeromebarre I've been informed from the OBS team that the agreed upon plan is to merge this PR as is, and revert the tropomi converter to the current develop version if necessary (before the Fortran API shows up). Is that your understanding? Are you okay with merging as is? |
|
Yes this refers to the averaging kernel and pressure vertices arrays and other ancillary variables such as the apriori quantities. Merging the tropomi converter as is as this will produce useless IODA files, i.e. breaking the converter. We will most likely have to revert most of the changes made but at this point it now feels that we ought to merge as is otherwise we'll waste even more time and energy on this. Thanks for checking anyway. |
|
@jeromebarre thank you for you generous patience and understanding. |
|
I'm going to merge now. |
6 participants
Description
This PR again updates the top-level sprint branch to incorporate changes from develop (I think almost entirely PR1100). With this update, we should be back on track to fixing the broken ctests related to naming convention changes so we can move ahead with the big push to get all sprint branches (UFO and IODA and all associates) to merge into develop in the next 2 weeks. There will be about 15 ctests not yet working, mostly BUFR - continuing work in another branch.
Issue(s) addressed
A separate one was not created.
Acceptance Criteria (Definition of Done)
Fixing as many ctests as possible.
Dependencies
There is a corresponding PR in ioda repo for the validation/ObsSpace.yaml
ioda PR 868
Impact
Keeping in sprint branch until ready for merge to develop.