Skip to content

Add GLODAP DataWrangling module#302

Open
lgloege wants to merge 21 commits into
NumericalEarth:mainfrom
CDR-Forecasting-Stack:main
Open

Add GLODAP DataWrangling module#302
lgloege wants to merge 21 commits into
NumericalEarth:mainfrom
CDR-Forecasting-Stack:main

Conversation

@lgloege

@lgloege lgloege commented May 27, 2026

Copy link
Copy Markdown

This PR adds the GLODAP gridded climatology to NumericalEarth for initializing BGC fields. GLODAP does not require any authentication and the data is distributed under CC BY 4.0 license.

@simone-silvestri

@navidcy navidcy added the data wrangling 🗃️ JRA55, ECCO, ERA5, and friends label May 27, 2026
@navidcy navidcy self-requested a review May 27, 2026 20:51
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated

@navidcy navidcy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I sprinkled a few comments

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You need to add this file in the src/DataWrangling/DataWrangling.jl, otherwise NumericalEarth is oblivious to it.

Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
@codecov

codecov Bot commented May 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 74 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/DataWrangling/GLODAP/GLODAP.jl 0.00% 74 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
lgloege and others added 16 commits June 1, 2026 10:34
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
)

# Dataset types
abstract type GLODAPDataset end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
abstract type GLODAPDataset end
abstract type GLODAPDataset <:AbstractStaticDataset end

from what I understand this product does not have multiple dates right?

Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
Comment on lines +169 to +170
# Custom retrieve_data: GLODAP NetCDF files contain Missing values (from _FillValue)
# which must be converted to NaN before the GPU kernel in set_metadata_field!.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is already handled in the data mangling of retrieve_data right?

@simone-silvestri simone-silvestri Jun 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indeed, here

# `read_data(data, i, j, k, region, mangling, FT)` returns the file value at
# the grid's (i, j, k) as `FT`, with `Missing` converted to NaN.
@inline read_data(data, i, j, k, ::Nothing, mangling, FT) = nan_convert_missing(FT, mangle(i, j, k, data, mangling))
@inline read_data(data, i, j, k, b::BoundingBoxOffset, mangling, FT) = nan_convert_missing(FT, mangle(i + b.di, j + b.dj, k, data, mangling))
@inline read_data(data, _, _, k, c::ColumnInfo, mangling, FT) = blend(c.ℑ, data, c, k, mangling, FT)

I think this whole retrieve_data method can be removed

Comment thread src/DataWrangling/GLODAP/GLODAP.jl Outdated
@simone-silvestri

Copy link
Copy Markdown
Member

Looks good! I have left some comments. Can you also add one unit test for the metadata creation / handling? (We can probably avoid download tests for now)

lgloege and others added 3 commits June 5, 2026 12:03
Co-authored-by: Simone Silvestri <silvestri.simone0@gmail.com>
Co-authored-by: Simone Silvestri <silvestri.simone0@gmail.com>
Co-authored-by: Simone Silvestri <silvestri.simone0@gmail.com>
@simone-silvestri

Copy link
Copy Markdown
Member

Looks like everything has been addressed! @lgloege if you can resolve the conflicts then add a unit test (just test the metadata creation, not the download) we can then merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data wrangling 🗃️ JRA55, ECCO, ERA5, and friends

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants