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

Refactoring Coriolis implementation and semantics #1904

Closed
tomchor opened this issue Jul 28, 2021 · 3 comments
Closed

Refactoring Coriolis implementation and semantics #1904

tomchor opened this issue Jul 28, 2021 · 3 comments

Comments

@tomchor
Copy link
Collaborator

tomchor commented Jul 28, 2021

I'm opening this issue based off of the long discussion about semantics and implementation of the Coriolis acceleration in #1892.

@glwagner correct me if I'm wrong in the description of the issues since I'm not sure I 100% understand them.

Currently we have (with some oversimplifcation):

  • FPlane
  • ConstantCartesianCoriolis
  • BetaPlane
  • NonTraditionalBetaPlane
  • HydrostaticSphericalCoriolis

Which is kind of a lot. Two issues that are not separate from each other are

  • The names don't have much consistency between themselves and are at times a bit obscure (a new user might ask what is a non-traditional β-plane, or a cartesian coriolis)
  • In the current system we need to have different abstractions for Coriolis accelerations (hence the large number of abstractions) for different coordinate systems because we need to store different data for calculations in different grids

A way to improve on both issues might be to have only one or two abstractions for background rotation only. So instead of storing, for example, each cartesian component of f for ConstantCartesianCoriolis or just the rotation rate and a scheme for HydrostaticSphericalCoriolis, we would only need to store a rotation_axis and a rotation_rate (or equivalent; plus possibly the scheme?).

The difficulties I could perceive are that different grids require different calculations so we'd need to dispatch on grid type (such as here

@inline φᶠᶠᵃ(i, j, k, grid::RegularLatitudeLongitudeGrid) = @inbounds grid.φᵃᶠᵃ[j]
@inline φᶠᶠᵃ(i, j, k, grid::ConformalCubedSphereFaceGrid) = @inbounds grid.φᶠᶠᵃ[i, j]

and also that the calculation apparently needs to take into account whether or not the flow is hydrostatic, so we may need to dispatch on model too? (Plus some possible complications about the pressure solver.)

CC @glwagner @francispoulin

@glwagner
Copy link
Member

I think what we need to do is to distinguish between "hydrostatic" (as a proxy for "thin aspect ratio") and "nonhydrostatic" (non-thin-aspect ratio) approximations.

Then our API would provide types for Coriolis forced induced by rotation at a constant rate:

  • NonhydrostaticCoriolis
  • HydrostaticCoriolis

By storing rotation_axis (a 3-tuple in the coordinate system associated with the user-specified grid) and either a rotation_rate or total magnitude f, both of these types can be "coordinate-system agnostic", and work with either a spherical or Cartesian coordinate system (the two we plan to support moving forward).

Then there are two more types associated with the so-called "beta" plane. These could keep their original name, or we can change "NonTraditionalBetaPlane to NonhydrostaticBetaPlane.

@navidcy has argued that it's worthwhile to keep FPlane and BetaPlane as is because of their familiarity and ubiquity in oceanography. I think I agree with that.

@glwagner
Copy link
Member

The difficulties I could perceive are that different grids require different calculations so we'd need to dispatch on grid type

I think this is ok. We can dispatch on both the grid (and its associated coordinate system) and typeof(coriolis) to infer the Coriolis model. I think such a design is good because it is code-efficient.

@glwagner
Copy link
Member

I think this is done

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

No branches or pull requests

2 participants