[rrfs-mpas-jedi] Replace JEDI obs offline domain check with a JEDI Polygon Check pre-filter.#1293
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
SamuelTrahanNOAA
changed the title
Polygon Check pre-filter.
| cp jedivar.yaml no_polygon_jedivar.yaml | ||
| "${USHrrfs}/yamlify_domain_edge.py" -g "invariant.nc" -i '' > polygon.yaml | ||
| cat polygon.yaml no_polygon_jedivar.yaml > jedivar.yaml | ||
| fi |
There was a problem hiding this comment.
@SamuelTrahanNOAA Thanks for getting this ready for retro testing.
I understand that this is still a draft PR. But a heads up that the polygon generation will only need to run once for each domain in its lifetime. So we will NOT include lines 139-143 in exrrfs_jedivar.sh, but instead run it manually once and then put the results somewhere (either as a fix file or as a exp setting option).
There was a problem hiding this comment.
Perhaps we can do this?
- Add a variable
$BOUNDARY_POLYGON_YAML - If that variable is set to a valid path, use that file,
- Otherwise, automatically generate it.
A side-effect of the automatic generation fallback is to give you the polygon boundary file you need to put in parm/
There was a problem hiding this comment.
I will test your branch and then talk further with you on this. Thanks!
|
@SamuelTrahanNOAA could you update https://github.com/NOAA-EMC/RDASApp first and have a ctest in RDASApp? |
| cp ../_workaround_/PolygonCheck/ObsPolygonCheck.h sorc/ufo/src/ufo/filters/ObsPolygonCheck.h | ||
| cp ../_workaround_/PolygonCheck/instantiateObsFilterFactory.h sorc/ufo/src/ufo/instantiateObsFilterFactory.h | ||
| cp ../_workaround_/PolygonCheck/filtersCMakeLists.txt sorc/ufo/test/testinput/unit_tests/filters/CMakeLists.txt | ||
| cp ../_workaround_/PolygonCheck/polygon_check.yaml sorc/ufo/test/testinput/unit_tests/filters/polygon_check.yaml |
There was a problem hiding this comment.
Do we need to include the unit tests into the workflow?
Usually, we don't do ctests in the workflow.
Also, could you add comment information before lines 68? Similar to lines 48-49? Thanks!
There was a problem hiding this comment.
Those lines should not be merged. Also, the PolygonCheck directory should not be merged. They're present only to facilitate practical testing of the feature.
I included the unit tests so the ufo/ directory is identical to my branch in the ufo PR. That makes it easier to confirm they're in sync.
There was a problem hiding this comment.
We don't know how long it will take for your UFO PR to get merged. Sometimes, it may take a few months.
It is very likely that we will use the workaround for a while before the UFO PR gets merged and RDASApp gets sync'ed and updated.
|
@ShunLiu-NOAA - I'd like to wait for the ufo PR to be merged before making PRs at higher levels. This PR is needed for practical tests of the change in a workflow. Also, there's already a unit test for the filter inside RDASApp/sorc/ufo in its PR. The unit test is under review, along with the filter. |
MatthewPyle-NOAA
changed the title
Polygon Check pre-filter.Polygon Check pre-filter.
3 participants
DESCRIPTION OF CHANGES:
Replaces the offline JEDI obs domain check with a domain check in a JEDI prefilter.
DEPENDENCIES:
The new filter isn't in JEDI UFO yet; it is here:
Until the filter PR is merged, there are equivalent
sorc/_workarounds_changes. They're insorc/build.rdasandsorc/_workarounds_/PolygonCheck. They should be removed and replaced by updated JEDI code before this PR is merged.TESTS CONDUCTED:
Three levels of testing:
I'm waiting for more thorough independent testing. That's one of the reasons why this is a draft.
Machines/Platforms:
Affects all machines/platforms for configurations that use MPAS JEDI.
Test cases:
All cases that run MPAS JEDI will use this change.
ISSUE: