Skip to content

Add Rossby localization documentation#1238

Open
shlyaeva wants to merge 1 commit into
developfrom
feature/rossbyloc_doc
Open

Add Rossby localization documentation#1238
shlyaeva wants to merge 1 commit into
developfrom
feature/rossbyloc_doc

Conversation

@shlyaeva
Copy link
Copy Markdown
Collaborator

Description

All in the title

Checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have run the unit tests before creating the PR

Copy link
Copy Markdown
Contributor

@travissluka travissluka left a comment

Choose a reason for hiding this comment

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

ugh, who put that Rossby radius localization in there without any associated documentation!?!? Whoever it was, that person no longer deserves to oversee soca!!

Copy link
Copy Markdown
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

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

Doc looks good, but there's an issue with the equation rendering:
loc-doc-render

When that happens, I ask my friend to make the markdown equation to render on gihub, it usually does the trick.

Copy link
Copy Markdown
Contributor

@travissluka travissluka left a comment

Choose a reason for hiding this comment

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

I'm rescinding my previous approval:
Now that sequential EnKF is merged in we should add a short compatibility note? A user who picks Rossby loc for EAKF will hit a runtime ABORT from ObsLocRossby::computeLocalization(Point3, Point3).

Suggested addition:

Solver compatibility

Rossby localization currently works only with LETKF / GETKF. It is not yet supported with sequential EnKF solvers (e.g. EAKF), which require obs–obs localization. rossby_radius is only defined on the model grid, not at arbitrary obs pairs. Invoking it from a sequential solver aborts at runtime.

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.

3 participants