Skip to content

Enable reading from Pierre's unified BGC dataset#274

Merged
NoraLoose merged 51 commits into
CWorthy-ocean:mainfrom
NoraLoose:new-bgc-dataset
Apr 8, 2025
Merged

Enable reading from Pierre's unified BGC dataset#274
NoraLoose merged 51 commits into
CWorthy-ocean:mainfrom
NoraLoose:new-bgc-dataset

Conversation

@NoraLoose
Copy link
Copy Markdown
Collaborator

@NoraLoose NoraLoose commented Mar 13, 2025

This PR enables reading from Pierre's "unified" BGC dataset for the classes

  • InitialConditions
  • BoundaryForcing
  • SurfaceForcing

Since the unified BGC dataset does not contain all necessary MARBL BGC variables, the missing BGC variables need to be filled in (see #273).

Internal changes

  • Some refactoring of InitialConditions, BoundaryForcing, SurfaceForcing to accommodate optional variable names (which the unified BGC dataset requires)
  • Added the UnifiedDataset class (+ two subclasses, UnifiedBGCDataset and UnifiedBGCSurfaceDataset) with the required post-processing steps
  • Added functions that fill in missing BGC variables after regridding onto ROMS grid
  • Some refactoring of the Dataset class:
    • Refactored choose_subdomain method to handle concatenation in longitude direction across both ends (upper + lower) correctly
    • Added attribute needs_lateral_fill to Dataset class, which is set to True by default. Pierre's unified BGC dataset has land values already filled in, so we set UnifiedDataset.needs_lateral_fill = False
    • Cleaned up docstrings of subclasses of Dataset to avoid duplication
    • Changes to handle fractional days in climatologies correctly (necessary for unified BGC dataset)
  • Created new validation test data to test reproducible results with the unified BGC data
  • Added new tests to CI test suite to verify that using unified BGC data works as expected

Checklist

  • Closes How to fill in missing BGC variables? #273
  • Tests added
  • Passes pre-commit run --all-files
  • Changes are documented in docs/releases.md
  • New functions/methods are listed in docs/api.rst
  • New functionality has documentation

@NoraLoose NoraLoose changed the title [WIP] Enable reading from Pierre's unified BGC dataset Enable reading from Pierre's unified BGC dataset Mar 18, 2025
Comment thread roms_tools/setup/utils.py
Comment thread roms_tools/setup/utils.py
return bgc_data


def compute_missing_surface_bgc_variables(bgc_data):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@abigale-wyatt, could you review the code for this function as well and check for the right units?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@abigale-wyatt did you have a look at this function as well? I think the units are wrong (see my comments in this function).

@NoraLoose NoraLoose requested a review from abigale-wyatt March 18, 2025 15:42
Comment thread roms_tools/setup/utils.py Outdated
Comment thread roms_tools/setup/utils.py Outdated
Comment thread roms_tools/setup/utils.py
Copy link
Copy Markdown

@abigale-wyatt abigale-wyatt left a comment

Choose a reason for hiding this comment

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

I added values for NH4 and DOC. The units on everything are correct.

@NoraLoose
Copy link
Copy Markdown
Collaborator Author

I added values for NH4 and DOC. The units on everything are correct.

Awesome, thanks so much. I addressed your comments in the commit d79d0da

@NoraLoose
Copy link
Copy Markdown
Collaborator Author

@abigale-wyatt I left one more question for you in the code.

@NoraLoose
Copy link
Copy Markdown
Collaborator Author

@abigale-wyatt I will merge this PR to avoid future merge conflicts, and open a separate issue to get the default values of NOx and NHy right.

@NoraLoose NoraLoose merged commit 3d43673 into CWorthy-ocean:main Apr 8, 2025
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.

How to fill in missing BGC variables?

2 participants