Skip to content

Adding restoring forces DIC/Alk#619

Open
smaticka wants to merge 29 commits into
mainfrom
dic_alk_restoring
Open

Adding restoring forces DIC/Alk#619
smaticka wants to merge 29 commits into
mainfrom
dic_alk_restoring

Conversation

@smaticka

@smaticka smaticka commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

This PR adds the ROMS-Tool's functionality to make restoring forces files for DIC and ALK, using NOAA's OceanSODA data.

The dataset that works is:

  • A single file (/anvil/projects/x-ees250129/Datasets/OceanSODA-ETHZ/OceanSODA_ETHZ-v2025.OCADS.01-1982-2024.nc) that contains NOAA's OceanSODA 2025 data. 1 degree resolution (variables dic and talk are needed, both in units of umol/kg).

The SurfaceForcing class type = restoring needs to be called, and sDIC AND sALK need to be specified in restoring_forces in order to create the file. both need to be included, or neither. If sss (salinity) is too be called, it needs to be called separately from sDIC and sALK since they have different time dimensions (monthly vs climatology).

  • Tests added
  • Passes pre-commit run --all-files
  • Changes are documented in docs/releases.md
  • change download url back to main before merge
  • merge roms-tools-test-data: Add reduced SODA data roms-tools-test-data#34
  • New functionality has documentation
  • Add a description in ROMS-Tools Datasets (download instructions)
  • in notable code changes channel, mention gsw added to env. need to update env.

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.43%. Comparing base (5bc2948) to head (4e03bc5).

Files with missing lines Patch % Lines
roms_tools/setup/surface_forcing.py 63.15% 8 Missing and 6 partials ⚠️
roms_tools/datasets/lat_lon_datasets.py 86.11% 3 Missing and 2 partials ⚠️
@@            Coverage Diff             @@
##             main     #619      +/-   ##
==========================================
- Coverage   88.53%   88.43%   -0.11%     
==========================================
  Files          28       28              
  Lines        7073     7141      +68     
  Branches     1242     1258      +16     
==========================================
+ Hits         6262     6315      +53     
- Misses        467      477      +10     
- Partials      344      349       +5     
Flag Coverage Δ
glorys-regional-invariance 34.25% <23.68%> (-0.08%) ⬇️
with-dask-1 79.02% <23.68%> (-0.51%) ⬇️
with-dask-2 49.86% <75.00%> (+0.26%) ⬆️
with-dask-and-xesmf-1 83.16% <23.68%> (-0.55%) ⬇️
with-dask-and-xesmf-2 52.87% <75.00%> (+0.24%) ⬆️
with-streaming 34.05% <23.68%> (-0.08%) ⬇️
without-dask 83.30% <75.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
roms_tools/datasets/download.py 100.00% <ø> (ø)
roms_tools/setup/utils.py 87.03% <100.00%> (+0.05%) ⬆️
roms_tools/datasets/lat_lon_datasets.py 89.85% <86.11%> (-0.18%) ⬇️
roms_tools/setup/surface_forcing.py 82.48% <63.15%> (-1.47%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@smaticka smaticka marked this pull request as ready for review June 23, 2026 01:06
@smaticka smaticka requested a review from kthyng June 24, 2026 19:48
@smaticka

smaticka commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

The PR was tested by making a restoring forces file for the GoM, and running ROMS for 1 month (Jan 2000). The restoring forces file was also compared to the data used to make it as a gut check.

These are the initial conditions (from GLODAP) and the restoring forces (from SODA), and their differences:
image

Based on these differences, we can calculate an estimate of the expected flux in the model (7.777/(100*86400) * (IC - restoring)):
image

The 'expected flux' (above) values (order 1e-4) match well with the output diagnostics of restoring force fluxes from the model (below):
image

image

The flux values are large that what Pierre saw with the Atlantic (1e-6), but we attribute the GoM fluxes being large to the large difference between the IC and the applied restoring forces, and because the model was only run for 1 month so there wasn't much time for the model to spin up and adjust.

Additionally, the file produced by ROMS-Tools was compared to the sub-sampled portion of the source file used to make the restoring forces, in order to make sure they are of similar magnitude and RT does what we expect it to do. Note, the below plots are from a different domain (the one used in the SurfaceForcing notebook).
DIC from the source (OceanSoda data file), which were lazy converted using ds['dic']*1025/1000:
image

DIC calculated from ROMS-Tools:
image

The values fall within a similar range that we'd expect.

@blsaenz blsaenz left a comment

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.

Just one thing I found, then I think good to go! I have not looked at the ipynb, will do that in a sec.


return ds

def post_process(self) -> None:

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.

I have not fully checked, but this smells not dask lazy, because the .where is called outside the dask framework and xarray will request the data. Maybe replace with something like this:

def post_process(self) -> None:
    """
    Processes SODAv2025 data values as follows:
    - drop the year coordinate
    - Apply a mask to the dataset based on locations of NaN values.
    """
    self.ds = self.ds.drop_vars("year")
    condition = self.ds["dic"].isnull().any(dim=self.dim_names["time"])
    self.ds["mask"] = xr.where(condition, 0, 1)

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.

I tested your changes in the surface_forcing notebook, and they appear to be similar or the old method ever so slightly faster... I tested each 3 times bc Anvil can be unpredictable.

using code change suggestion:

    condition = self.ds["dic"].isnull().any(dim=self.dim_names["time"])
    self.ds["mask"] = xr.where(condition, 0, 1)

these were the specs:
1)
CPU times: user 3.28 s, sys: 711 ms, total: 3.99 s
Wall time: 45.1 s
2)
CPU times: user 2.96 s, sys: 498 ms, total: 3.45 s
Wall time: 9.07 s
3)
CPU times: user 2.85 s, sys: 478 ms, total: 3.33 s
Wall time: 9.32 s

using the original code, these were the specs:
1)
CPU times: user 3.17 s, sys: 618 ms, total: 3.79 s
Wall time: 30 s
2)
CPU times: user 2.94 s, sys: 547 ms, total: 3.49 s
Wall time: 8.61 s
3)
CPU times: user 3.18 s, sys: 649 ms, total: 3.83 s
Wall time: 8.72 s

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.

2 participants