Skip to content

Updating to latest JEDI Dev (22May2025) #555

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

Merged
merged 5 commits into from
May 22, 2025

Conversation

rtodling
Copy link
Contributor

@rtodling rtodling commented May 20, 2025

Description

This introduce changes to how the background error covariance term based on GSIBEC is handled in the latest version of JEDI. The out-block now handle the interpolation in a different way - needing different parameters including the number of lat and lon grid points that JEDI has not referenced so far. I am using this requirement to generalize the handling of the static files so that choice of horizontal and vertical grids are more encompassing.

Dependencies

Latest JEDI version (22 May 2025)

Impact

Shouldn't be any

@rtodling rtodling changed the title Updating to latest JEDI Dev (19May2025) Updating to latest JEDI Dev (16May2025) May 21, 2025
@rtodling
Copy link
Contributor Author

@mranst could you pls launch a tier1 test on this? Not sure how to do it on my end.

@rtodling rtodling requested review from Dooruk and mranst May 21, 2025 23:04
@Dooruk
Copy link
Collaborator

Dooruk commented May 22, 2025

@rtodling would your PR work with February 6th version?

@mranst
Copy link
Collaborator

mranst commented May 22, 2025

Looks like the tier1 tests passed
https://github.com/GEOS-ESM/swell/actions/runs/15174089059

@Dooruk Dooruk requested a review from Copilot May 22, 2025 13:50
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Introduces support for parameterized GSIBEC grid sizes in line with JEDI Dev (16 May 2025).

  • Adds gsibec_nlats and gsibec_nlons TaskQuestions for specifying GSIBEC grid dimensions.
  • Propagates the new parameters through rendering, task execution, and suite defaults.
  • Updates YAML templates to incorporate the latitude/longitude counts in file paths and interpolation blocks.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/swell/utilities/render_jedi_interface_files.py Registered gsibec_nlats and gsibec_nlons as JEDI template keys
src/swell/utilities/question_defaults.py Added gsibec_nlats and gsibec_nlons TaskQuestion classes
src/swell/tasks/task_questions.py Included the new questions in the TaskQuestions enum
src/swell/tasks/stage_jedi.py Passed gsibec_nlats/nlons into JEDI rendering for stage task
src/swell/tasks/run_jedi_variational_executable.py Added the new grid parameters to the variational executable render
src/swell/tasks/run_jedi_fgat_executable.py Added the new grid parameters to the FGAT executable render
src/swell/suites/3dvar_atmos/suite_config.py Set default values for gsibec_nlats/nlons in the 3DVar Atmos suite
src/swell/configuration/jedi/interfaces/geos_atmosphere/task_questions.yaml Defined defaults and options for GSIBEC lat/lon counts
src/swell/configuration/jedi/interfaces/geos_atmosphere/model/stage_cycle.yaml Updated static file paths to include the new grid size keys
src/swell/configuration/jedi/interfaces/geos_atmosphere/model/stage.yaml Updated staging file paths to include the new grid size keys
src/swell/configuration/jedi/interfaces/geos_atmosphere/model/background_error.yaml Reorganized the outer interpolation block to use custom grid specs
Comments suppressed due to low confidence (2)

src/swell/configuration/jedi/interfaces/geos_atmosphere/model/stage_cycle.yaml:7

  • There's a double slash between gsibec and 1.0.1; remove the extra slash to match the intended path.
-      - ['{{swell_static_files}}/jedi/interfaces/geos_atmosphere/gsibec//1.0.1/{{gsibec_configuration}}_l{{vertical_resolution}}x{{gsibec_nlons}}y{{gsibec_nlats}}.nml', '{{cycle_dir}}/fv3-jedi/gsibec/']

src/swell/utilities/question_defaults.py:615

  • There's no existing test coverage for the new gsibec_nlats/gsibec_nlons questions. Consider adding unit tests to verify defaults and rendering behavior.
class gsibec_nlats(TaskQuestion):

@Dooruk Dooruk changed the title Updating to latest JEDI Dev (16May2025) Updating to latest JEDI Dev (22May2025) May 22, 2025
@Dooruk
Copy link
Collaborator

Dooruk commented May 22, 2025

@mranst how critical are the widget_type: WType = WType.INTEGER entries?

@mranst
Copy link
Collaborator

mranst commented May 22, 2025

@mranst how critical are the widget_type: WType = WType.INTEGER entries?

It will attempt to check if the answer is an integer or not, so if the answer is always going to be an integer, I'd recommend it

@mranst
Copy link
Collaborator

mranst commented May 22, 2025

Have the pinned versions been updated?

Dooruk
Dooruk previously approved these changes May 22, 2025
Copy link
Collaborator

@Dooruk Dooruk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, waiting for tier1 tests after the final changes

@Dooruk Dooruk self-requested a review May 22, 2025 19:32
Copy link
Collaborator

@Dooruk Dooruk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, updated the pins 🪡

@Dooruk Dooruk merged commit b642cc0 into develop May 22, 2025
2 checks passed
@Dooruk Dooruk deleted the feature/rtodling/gsibec_oops_inter_upd branch May 22, 2025 20:21
@Dooruk Dooruk linked an issue May 22, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump JEDI version to May 22nd, 2025
3 participants