Skip to content

Add marine job and make it work on HPC#43

Merged
guillaumevernieres merged 25 commits into
mainfrom
feature/marine_task
Apr 2, 2025
Merged

Add marine job and make it work on HPC#43
guillaumevernieres merged 25 commits into
mainfrom
feature/marine_task

Conversation

@guillaumevernieres
Copy link
Copy Markdown
Contributor

@guillaumevernieres guillaumevernieres commented Apr 1, 2025

Copy link
Copy Markdown
Contributor

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

This PR adds support for processing marine observation data on HPC systems, addressing issues #38 and #42. Key changes include:

  • A new Python module (marine_prepobs.py) that implements marine observation processing with multiprocessing.
  • New test scripts and modifications to the CI workflows to include marine dump testing.
  • Updates to the configuration file and build scripts for HPC-specific settings.

Reviewed Changes

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

Show a summary per file
File Description
ush/python/pyobsforge/task/marine_prepobs.py Adds marine observation processing task with multiprocessing support.
scripts/tests/test_exobsforge_global_marine_dump.py Introduces tests for the marine dump functionality.
scripts/tests/conftest.py Provides environment variable fixtures for test isolation.
scripts/exobsforge_global_marine_dump.py Adds the marine dump script to initialize and run the new task.
parm/config.yaml Updates configuration parameters for marine dump and HPC settings.
README.md Updates documentation and build instructions.
.github/workflows/pytest.yaml Modifies test workflow to include marine dump tests.
.github/workflows/build.yaml Adjusts build workflow to set up dependencies and run tests in Docker.
Files not reviewed (6)
  • jobs/JOBSFORGE_GLOBAL_MARINE_DUMP: Language not supported
  • jobs/rocoto/marinedump.sh: Language not supported
  • parm/nc2ioda/nc2ioda.yaml.j2: Language not supported
  • parm/obsforge_rocoto_template.xml.j2: Language not supported
  • ush/jjob_header.sh: Language not supported
  • ush/load_obsforge_modules.sh: Language not supported
Comments suppressed due to low confidence (3)

ush/python/pyobsforge/task/marine_prepobs.py:36

  • [nitpick] Consider replacing the print statement with logger.debug(self.task_config) for consistency with the rest of the logging in production code.
print(self.task_config)

ush/python/pyobsforge/task/marine_prepobs.py:182

  • [nitpick] The multiline f-string used for logging the error may introduce unintended whitespace; consider combining the string on one line or using parentheses to join multiple strings for clarity.
logger.warning(f"ioda converter failed with error {e}, \n                return code {e.returncode}")

parm/config.yaml:39

  • [nitpick] Consider renaming the key 'min number of obs' to 'min_number_of_obs' for consistency with other configuration keys.
min number of obs: 10

Copy link
Copy Markdown
Contributor

@CoryMartin-NOAA CoryMartin-NOAA left a comment

Choose a reason for hiding this comment

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

a few comments

Comment thread README.md Outdated
Comment on lines +34 to +35
export HOMEobsforge=<path to obsForge>
export PYTHONPATH="${PYTHONPATH}:${HOMEobsforge}/sorc/wxflow/src:${HOMEobsforge}/ush/python"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

instead: source ush/of_setup.sh


<!--
============================================
Task: gfs_marine_dump
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why start with gfs and not gdas?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no reason really, just a prototype for now. I'll add gdas the next time around.

Comment thread ush/jjob_header.sh
yaml_config = Jinja(jinja_template, context).render
nc2ioda_yaml = join(self.task_config['DATA'], obs_space, f"{obs_space}_nc2ioda.yaml")
with open(nc2ioda_yaml, "w") as fho:
fho.write(yaml_config)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why are you not using wxflow methods to write the YAML?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ha! copy/paste without thinking.

print("Standard Output:")
print(result.stdout)

# Optionally, print the standard error
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it optionally???

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ha ... no ...

Copy link
Copy Markdown
Contributor

@CoryMartin-NOAA CoryMartin-NOAA left a comment

Choose a reason for hiding this comment

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

bon!

@guillaumevernieres guillaumevernieres merged commit 50a083a into main Apr 2, 2025
@guillaumevernieres guillaumevernieres deleted the feature/marine_task branch April 2, 2025 14:25
CoryMartin-NOAA added a commit that referenced this pull request Oct 21, 2025
…sts (#12)

1. Simplified ctest structure.
My initial version is very stupid and inefficient!
e.g. initial ctest list for test_obsforge_satobs_satwnd_amv_seviri looks
like the following:
```
  Test  #38: test_obsforge_satobs_satwnd_amv_seviri_m8_bufr2netcdf
  Test  #39: test_obsforge_satobs_satwnd_amv_seviri_m8_bufr2netcdf_mpi4
  Test  #40: test_obsforge_satobs_satwnd_amv_seviri_m8_script2netcdf
  Test  #41: test_obsforge_satobs_satwnd_amv_seviri_m8_script2netcdf_mpi4
  Test  #42: test_obsforge_satobs_satwnd_amv_seviri_m8_bufr4backend
  Test  #43: test_obsforge_satobs_satwnd_amv_seviri_m8_bufr4backend_mpi4
  Test  #44: test_obsforge_satobs_satwnd_amv_seviri_m8_script4backend
  Test  #45: test_obsforge_satobs_satwnd_amv_seviri_m8_script4backend_mpi4
  Test  #46: test_obsforge_satobs_satwnd_amv_seviri_m9_bufr2netcdf
  Test  #47: test_obsforge_satobs_satwnd_amv_seviri_m9_bufr2netcdf_mpi4
  Test  #48: test_obsforge_satobs_satwnd_amv_seviri_m9_script2netcdf
  Test  #49: test_obsforge_satobs_satwnd_amv_seviri_m9_script2netcdf_mpi4
  Test  #50: test_obsforge_satobs_satwnd_amv_seviri_m9_bufr4backend
  Test  #51: test_obsforge_satobs_satwnd_amv_seviri_m9_bufr4backend_mpi4
  Test  #52: test_obsforge_satobs_satwnd_amv_seviri_m9_script4backend
  Test  #53: test_obsforge_satobs_satwnd_amv_seviri_m9_script4backend_mpi4
  Test  #54: test_obsforge_satobs_satwnd_amv_seviri_m10_bufr2netcdf
  Test  #55: test_obsforge_satobs_satwnd_amv_seviri_m10_bufr2netcdf_mpi4
  Test  #56: test_obsforge_satobs_satwnd_amv_seviri_m10_script2netcdf
  Test  #57: test_obsforge_satobs_satwnd_amv_seviri_m10_script2netcdf_mpi4
  Test  #58: test_obsforge_satobs_satwnd_amv_seviri_m10_bufr4backend
  Test  #59: test_obsforge_satobs_satwnd_amv_seviri_m10_bufr4backend_mpi4
  Test  #60: test_obsforge_satobs_satwnd_amv_seviri_m10_script4backend
  Test  #61: test_obsforge_satobs_satwnd_amv_seviri_m10_script4backend_mpi4
  Test  #62: test_obsforge_satobs_satwnd_amv_seviri_m11_bufr2netcdf
  Test  #63: test_obsforge_satobs_satwnd_amv_seviri_m11_bufr2netcdf_mpi4
  Test  #64: test_obsforge_satobs_satwnd_amv_seviri_m11_script2netcdf
  Test  #65: test_obsforge_satobs_satwnd_amv_seviri_m11_script2netcdf_mpi4
  Test  #66: test_obsforge_satobs_satwnd_amv_seviri_m11_bufr4backend
  Test  #67: test_obsforge_satobs_satwnd_amv_seviri_m11_bufr4backend_mpi4
  Test  #68: test_obsforge_satobs_satwnd_amv_seviri_m11_script4backend
  Test  #69: test_obsforge_satobs_satwnd_amv_seviri_m11_script4backend_mpi4
```

The simplified one looks like the following for satwnd_amv_seviri:
``` 
      Start 22: test_obsforge_satobs_satwnd_amv_seviri_bufr2netcdf
 7/38 Test #22: test_obsforge_satobs_satwnd_amv_seviri_bufr2netcdf ...........   Passed   16.69 sec
      Start 23: test_obsforge_satobs_satwnd_amv_seviri_bufr2netcdf_mpi4
 8/38 Test #23: test_obsforge_satobs_satwnd_amv_seviri_bufr2netcdf_mpi4 ......   Passed    9.47 sec
      Start 24: test_obsforge_satobs_satwnd_amv_seviri_script2netcdf
 9/38 Test #24: test_obsforge_satobs_satwnd_amv_seviri_script2netcdf .........   Passed   18.98 sec
      Start 25: test_obsforge_satobs_satwnd_amv_seviri_script2netcdf_mpi4
10/38 Test #25: test_obsforge_satobs_satwnd_amv_seviri_script2netcdf_mpi4 ....   Passed   13.31 sec
      Start 26: test_obsforge_satobs_satwnd_amv_seviri_bufr4backend
11/38 Test #26: test_obsforge_satobs_satwnd_amv_seviri_bufr4backend ..........   Passed   18.95 sec
      Start 27: test_obsforge_satobs_satwnd_amv_seviri_bufr4backend_mpi4
12/38 Test #27: test_obsforge_satobs_satwnd_amv_seviri_bufr4backend_mpi4 .....   Passed   11.18 sec
      Start 28: test_obsforge_satobs_satwnd_amv_seviri_script4backend
13/38 Test #28: test_obsforge_satobs_satwnd_amv_seviri_script4backend ........   Passed   22.38 sec
      Start 29: test_obsforge_satobs_satwnd_amv_seviri_script4backend_mpi4
14/38 Test #29: test_obsforge_satobs_satwnd_amv_seviri_script4backend_mpi4 ...   Passed   16.85 sec
```
The test structure is simplified in that the test for each data type,
and each configuration only needs to be tested once because the
`bufr_comp.sh` has been modified to handle comparisons of output files
from various satellite platforms for the same sensor type. (See
[bufr-query PR #53](NOAA-EMC/bufr-query#53))

2. Enable IASI tests
    - add tests in CMakeList.txt
    - add test references (testoutput) in obsForge-0.0.0.tgz on EMCRZDM
    - add test configuration YAMLs (testinput)
- mapping file and python configuration YAMLs are added in [SPOC
PR#25](NOAA-EMC/spoc#25)

---------

Co-authored-by: Cory Martin <cory.r.martin@noaa.gov>
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.

Move the scripts tests inside of the jcsda container instance Add the marine prep obs to the workflow

3 participants