Download crtm coefficient once and use it by crtm, ufo, fv3-jedi and mpas-jedi - #302
Conversation
…erty so UFO can use the same path to get the crtm coefficients
| endif() | ||
|
|
||
|
|
||
| set (UFO_CRTM_TESTFILES_PATH ${CRTM_COEFFS_PATH}/${CRTM_COEFFS_BRANCH}/fix) |
There was a problem hiding this comment.
CRTM is often used outside of JEDI. I wonder if it made sense to create a CMake option that tells the build system if this is a build in JEDI or standalone, and then set stuff like this. That might also help with the compiler flags discussion that @fmahebert and @BenjaminTJohnson had.
There was a problem hiding this comment.
How about naming this CRTM_TESTFILES_PATH ?
There's no need to prefix it with UFO. This way, it stays neutral and my comment from this morning can be ignored. Same in the UFO PR.
There was a problem hiding this comment.
Is the variable CRTM_COEFFS_PATH available when the if (EXISTS ${FIX_FILES_PATH}) code path is taken above (instead of the download code path) ?
There was a problem hiding this comment.
I was following the else code of (EXISTS ${FIX_FILE_PATH}), setting the CRTM_COEFFS_PATH.
| set( CRTM_COEFFS_BRANCH "fix_REL-3.1.2.0" ) | ||
|
|
||
| set(CRTM_COEFFS_PATH ${CMAKE_BINARY_DIR}/test_data/${PROJECT_VERSION}) | ||
| set(CRTM_COEFFS_PATH ${CMAKE_SOURCE_DIR}/test-data-release) |
There was a problem hiding this comment.
This is outside my expertise, perhaps better for @BenjaminTJohnson or @climbfuji — do we think using CMAKE_SOURCE_DIR will work well when CRTM is installed as an external library (vs as part of a compiled bundle), e.g. in the case of using spack-stack to provide a CRTM module?
Or is this question moot, because this is all in test/CMakeLists.txt, and this code path is probably not exercised in a CRTM-as-a-library context?
There was a problem hiding this comment.
This is actually a core part of the librification of CRTM and other JEDI components. We need to be able to run the ctests. I had to patch the CRTM code (and the UFO code) in order to make the stars line up. I can update the patch as needed in the Spack recipe, hence I don't see this as a big problem. The only thing I am adament about is to not prefix CRTM_TESTFILES_PATH with UFO_, because those testfiles are CRTM's testfiles and independent of whether UFO is being used or not?
| endif() | ||
|
|
||
|
|
||
| set (UFO_CRTM_TESTFILES_PATH ${CRTM_COEFFS_PATH}/${CRTM_COEFFS_BRANCH}/fix) |
There was a problem hiding this comment.
Is the variable CRTM_COEFFS_PATH available when the if (EXISTS ${FIX_FILES_PATH}) code path is taken above (instead of the download code path) ?
|
|
||
|
|
||
| set (CRTM_TESTFILES_PATH ${CRTM_COEFFS_PATH}/${CRTM_COEFFS_BRANCH}/fix) | ||
| message(STATUS "Set the file path to be seen by UFO at ${CRTM_TESTFILES_PATH} ") |
There was a problem hiding this comment.
@rajichidamb change this message to be model agnostic -- specifically, CRTM is primarily a stand-alone package/library, that UFO happens to use. So mentioning UFO here would be quite weird to most users.
develop did not contain release/REL-3.1.4: git merge-base --is-ancestor reported it as not an ancestor, with exactly one commit missing, 0f1ab81. That commit is a release stamp and nothing more. It bumps the project version from 3.1.3 to 3.1.4 in CMakeLists.txt, VERSION.cmake and test/CMakeLists.txt, and adds a README note about the coefficient-data location. The mechanism its subject line refers to, moving the default coefficient data to test-data-release, landed earlier in 9901de1 (PR #302) and has been on develop all along. There is no Fortran source in it, so this back-merge changes no library behaviour. Conflict-free by construction: since the 9901de1 merge base, develop has only touched .github/workflows/start-jedi-ci.yaml, and 0f1ab81 touches none of that. develop now reports version 3.1.4, which is the expected state after a release back-merge and will be superseded when 3.2.0 lands.
4 participants
Description
CRTM coefficient is downloaded twice when building using jedi-bundle, once by crtm module and then by ufo and saved in two different locations. Changing the download location to be in jedi-bundle and then used as a symlink in the build directory for testing, reduces the download of coefficient to only once.
Issue(s) addressed
Resolves #https://github.com/JCSDA-internal/ufo/issues/4085
Dependencies
List the other PRs that this PR is dependent on:
build-group=https://github.com/JCSDA-internal/ufo/pull/4123
Impact
Expected impact on downstream repositories:
Checklist