Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single column grid update_state! is missing the update_model_field_time_series! #3756

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

simone-silvestri
Copy link
Collaborator

required to force the model with a field time series.
This PR adds update_model_field_time_series! to the update_state! for single column grids and a test

@simone-silvestri
Copy link
Collaborator Author

Bonus, also the mask_immersed_field was missing. I don't expect it will be ever used (why using an immersed boundary grid in a single column?), however I have added a test for it. We can remove it if not needed.

@glwagner
Copy link
Member

glwagner commented Sep 3, 2024

Bonus, also the mask_immersed_field was missing. I don't expect it will be ever used (why using an immersed boundary grid in a single column?), however I have added a test for it. We can remove it if not needed.

Good to have because it might be useful for testing and also, its definitely best if the single column mode is identical (except for performance) to 3D mode.

@@ -37,6 +39,62 @@ const CAVD = ConvectiveAdjustmentVerticalDiffusivity
@test all(sic_model.velocities.u.data[1, 1, :] .≈ per_model.velocities.u.data[1, 1, :])
@test all(sic_model.velocities.v.data[1, 1, :] .≈ per_model.velocities.v.data[1, 1, :])
@test all(sic_model.tracers.c.data[1, 1, :] .≈ per_model.tracers.c.data[1, 1, :])

@info "Testing forcing a single column grid model with a FieldTimeSeries..."
Copy link
Member

Choose a reason for hiding this comment

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

it looks like this tests both single column and periodic

Copy link
Member

Choose a reason for hiding this comment

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

Another way to do this test would be to add a single column grid test alongside wherever update_model_field_time_series is already tested. That might be less test code / easier to maintain.

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