Add the marine task by guillaumevernieres · Pull Request #41 · NOAA-EMC/obsForge

guillaumevernieres · 2025-03-27T21:13:17Z

fixes Add the marine prep obs to the workflow #38

Copilot

Pull Request Overview

This PR introduces a new marine observation pre-processing task to support marine data assimilation and fixes issue #38.

Added MarineObsPrep task to create observation time windows and ingest GHRSST data files
Added tests for global marine dump functionality and updated configuration files
Extended GitHub workflow to run new tests

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
ush/python/pyobsforge/task/marine_prepobs.py	Introduces the MarineObsPrep task with initialization, execution, and finalize methods
scripts/tests/test_exobsforge_global_marine_dump.py	Adds test coverage for the marine dump script
scripts/tests/conftest.py	Sets up test environment variables and directories
scripts/tests/config.yaml	Provides necessary configuration for the marine dump tests
scripts/exobsforge_global_marine_dump.py	Implements a script to run the marine pre-processing task
parm/config.yaml	Updates configuration with marine dump parameters
.github/workflows/pytest.yaml	Extends CI to run tests in the scripts/tests directory

Files not reviewed (2)

jobs/JOBSFORGE_GLOBAL_MARINE_DUMP: Language not supported
jobs/rocoto/marinedump.sh: Language not supported

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

This PR implements a new marine observation task to address issue #38 by adding a new task for ingesting and processing marine observation data. Key changes include:

Creation of MarineObsPrep task in the pyobsforge package.
Addition of test scripts and configuration files for marine dumping and database ingestion.
Updates to CI workflow to run new tests.

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
ush/python/pyobsforge/task/marine_prepobs.py	New task implementation for marine observations.
scripts/tests/test_exobsforge_global_marine_dump.py	Test script for the new marine dump functionality.
scripts/tests/conftest.py	Fixture setup including environment variables and directory setup.
scripts/tests/config.yaml	Test configuration additions for marine dump settings.
scripts/exobsforge_global_marine_dump.py	Script initialization for combining configurations and executing the marine prep task.
parm/config.yaml	Production configuration update to include marinedump settings.
.github/workflows/pytest.yaml	Workflow update to run newly added tests.

Files not reviewed (2)

jobs/JOBSFORGE_GLOBAL_MARINE_DUMP: Language not supported
jobs/rocoto/marinedump.sh: Language not supported

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

This pull request introduces a new marine observation task for preprocessing marine data and adds corresponding tests, configurations, and workflow updates. Key changes include:

The addition of the MarineObsPrep task class to prepare and manage marine observations.
New tests and configuration files to support the marine dump functionality.
Workflow updates to run the new tests alongside existing ones.

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
ush/python/pyobsforge/task/marine_prepobs.py	Introduces the MarineObsPrep task that sets up time windows, merges configuration, and interfaces with the GHRSST database.
scripts/tests/test_exobsforge_global_marine_dump.py	Adds tests for the global marine dump script and creates an expected directory structure using a tree file.
scripts/tests/conftest.py	Provides session-scoped environment variable and test output configuration for integration tests.
scripts/tests/config.yaml	Supplies new configuration settings for marine dump alongside existing settings.
scripts/exobsforge_global_marine_dump.py	Implements a script that leverages MarineObsPrep to initialize, execute, and finalize marine observation processing.
parm/config.yaml	Updates configuration to include marinedump settings.
.github/workflows/pytest.yaml	Updates the workflow to run tests in the scripts/tests directory.

Files not reviewed (2)

jobs/JOBSFORGE_GLOBAL_MARINE_DUMP: Language not supported
jobs/rocoto/marinedump.sh: Language not supported

Comments suppressed due to low confidence (2)

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

[nitpick] The local variable name '_window_begin' is very similar to the key 'window_begin' that is later merged into self.task_config. Consider renaming it (e.g., to 'calc_window_begin') to improve clarity and avoid potential confusion.

_window_begin = add_to_datetime(self.task_config.current_cycle, -to_timedelta(f"{self.task_config['assim_freq']}H") / 2)

scripts/tests/test_exobsforge_global_marine_dump.py:60

The test depends on an external file ('dcom_tree.txt') being present relative to the test file. To improve test reliability, consider including a fixture that creates sample content for this file or embedding a test version directly within the test itself.

create_dcom(output_root="./dcom", dcom_tree_file=Path(__file__).parent / "dcom_tree.txt")

Copilot

Pull Request Overview

This PR introduces a new marine observation task for preprocessing observations and integrates it into the larger obsforge framework. Key changes include:

Creation of the MarineObsPrep class to support marine observation preprocessing.
Addition of new test scripts and configuration updates for validating the global marine dump workflow.
Updates to CI configuration to run the new tests.

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
ush/python/pyobsforge/task/marine_prepobs.py	Implements the MarineObsPrep class for marine observations
scripts/tests/test_exobsforge_global_marine_dump.py	Adds testing for the marine dump script using subprocess
scripts/tests/conftest.py	Sets up environment variables and directories for testing
scripts/tests/config.yaml	Provides configuration for the marine dump tests
scripts/exobsforge_global_marine_dump.py	New script to execute the marine observation task
parm/config.yaml	Adds marine dump configuration parameters
.github/workflows/pytest.yaml	Updates CI to include the new tests

Files not reviewed (2)

jobs/JOBSFORGE_GLOBAL_MARINE_DUMP: Language not supported
jobs/rocoto/marinedump.sh: Language not supported

Comments suppressed due to low confidence (2)

scripts/exobsforge_global_marine_dump.py:33

[nitpick] Consider renaming the variable 'MarineObs' to 'marine_obs' to follow Python naming conventions for variables.

MarineObs = MarineObsPrep(config)

scripts/tests/test_exobsforge_global_marine_dump.py:81

[nitpick] It would be beneficial to add assertions that verify expected output content or key side effects to further validate that the marine dump script behaves as intended.

assert result.returncode == 0

guillaumevernieres · 2025-03-28T14:47:36Z

closing for now.

…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>

guillaumevernieres added 5 commits March 26, 2025 16:58

added marine job and test for script

748e868

Merge branch 'main' into feature/marine_task

db54742

added pytests

ed54327

py norms

21c0b88

added pytest to gitrunner

a468fa0

guillaumevernieres requested review from CoryMartin-NOAA and Copilot March 28, 2025 12:08

Copilot AI reviewed Mar 28, 2025

View reviewed changes

Comment thread ush/python/pyobsforge/task/marine_prepobs.py Outdated

guillaumevernieres and others added 2 commits March 28, 2025 08:11

Update ush/python/pyobsforge/task/marine_prepobs.py

d3bd41b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

...

bf17e68

guillaumevernieres requested a review from Copilot March 28, 2025 12:27

Copilot AI reviewed Mar 28, 2025

View reviewed changes

Comment thread ush/python/pyobsforge/task/marine_prepobs.py Outdated

Update ush/python/pyobsforge/task/marine_prepobs.py

0d48782

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

guillaumevernieres requested a review from Copilot March 28, 2025 12:34

Copilot AI reviewed Mar 28, 2025

View reviewed changes

...

e1e0674

guillaumevernieres requested a review from Copilot March 28, 2025 12:39

Copilot AI reviewed Mar 28, 2025

View reviewed changes

wrong var name conv

b680985

guillaumevernieres closed this Mar 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the marine task#41

Add the marine task#41
guillaumevernieres wants to merge 10 commits into
mainfrom
feature/marine_task

guillaumevernieres commented Mar 27, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

guillaumevernieres commented Mar 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

guillaumevernieres commented Mar 27, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

guillaumevernieres commented Mar 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants