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

Initial commit of marine letkf stubs #1090

Merged
merged 3 commits into from
May 3, 2024
Merged

Conversation

AndrewEichmann-NOAA
Copy link
Collaborator

@AndrewEichmann-NOAA AndrewEichmann-NOAA commented May 2, 2024

Adds stubs and ctest for marine LETKF task. Co-dependent on global-workflow PR NOAA-EMC/global-workflow#2564

Addresses #1091

Copy link
Collaborator

@apchoiCMD apchoiCMD left a comment

Choose a reason for hiding this comment

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

Looks good-

@guillaumevernieres guillaumevernieres merged commit 435eb66 into develop May 3, 2024
5 checks passed
@guillaumevernieres guillaumevernieres deleted the feature/ocn-letkf branch May 3, 2024 17:28
logger = getLogger(__name__.split('.')[-1])


class MarineLETKF(Task):
Copy link
Contributor

Choose a reason for hiding this comment

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

I know Analysis task has a few (2) atmosphere specific methods, but the constructor for it sets up the window, etc. Also, there are generic methods to construct the JEDI yaml, copy observations, etc.
This class deviates from the framework that the other analysis tasks are developed around.
If there is a need to update Analysis task to be further accommodating of ocean/seaice requirements, it would be better to discuss and develop rather than have to revisit when there is a crunch.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's why we added you as a reviewer and that's what a PR is for @aerorahul, discuss.

Can you switch to using Analysis as parent class @AndrewEichmann-NOAA ? Also, while re-reading your code, I saw a few places where you are using recentering instead of letkf.

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.

5 participants