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.

Sign up for GitHub

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

Jump to bottom

update htlm related refs for OOPS PR "Hybrid TLM clean up" #1070

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from
Draft

update htlm related refs for OOPS PR "Hybrid TLM clean up" #1070

wants to merge 8 commits into from

Conversation

kbhargava
Copy link
Collaborator

@kbhargava kbhargava commented Sep 6, 2024 •
edited
Loading

Description

Update testrefs for https://github.com/JCSDA-internal/oops/pull/2734

Issue(s) addressed

Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues should always be created before starting work on a PR branch!)

Testing

Tested on orion with gnu that ctest pass

Dependencies

  • waiting on JCSDA-internal/oops/pull/2734

build-group=https://github.com/JCSDA-internal/oops/pull/2734

@kbhargava kbhargava requested review from travissluka, xian22 and tom-j-h and removed request for tom-j-h September 6, 2024 16:48
tom-j-h
tom-j-h approved these changes Sep 9, 2024
View reviewed changes
Copy link

@tom-j-h tom-j-h left a comment

Choose a reason for hiding this comment

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

Small differences. Some are "worse" but I don't think that's really true given this is just a basic software test. Thanks!

@travissluka
Copy link
Collaborator

@kbhargava you need to put

build-group=https://github.com/JCSDA-internal/oops/pull/2734

in the description so that the CI builds with the right branches

@kbhargava
Copy link
Collaborator Author

@kbhargava you need to put

build-group=JCSDA-internal/oops#2734

in the description so that the CI builds with the right branches

Thanks Travis. I did not know this. Added it in the description. Hopefully ctests pass now. :)

@kbhargava
Copy link
Collaborator Author

Small differences. Some are "worse" but I don't think that's really true given this is just a basic software test. Thanks!

Thanks!! Also given we have the issue of RMSE and unrealistic initial perturbations, I am not worried too much about these tests. We are working on more realistic case with larger ensemble size. That would be a better test of "science".

@travissluka travissluka added the coordinate merge Ready for merge but needs to be coordinated with other repos label Sep 16, 2024
travissluka
travissluka approved these changes Sep 16, 2024
View reviewed changes
Copy link
Collaborator

@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.

1e10... still a long way from 1e308, so 👍

@shlyaeva
Copy link
Contributor

Looks like the test reference needs merging with develop.

@kbhargava
Copy link
Collaborator Author

Looks like the test reference needs merging with develop.

Thanks for pinting thi @shlyaeva let me fix it.

@travissluka travissluka added the SOCA Sea-ice, Ocean, and Coupled Assimilation label Oct 25, 2024
@travissluka travissluka marked this pull request as draft October 30, 2024 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tom-j-h tom-j-h tom-j-h approved these changes

@travissluka travissluka travissluka approved these changes

@xian22 xian22 Awaiting requested review from xian22

Assignees

@kbhargava kbhargava

Labels
coordinate merge Ready for merge but needs to be coordinated with other repos SOCA Sea-ice, Ocean, and Coupled Assimilation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update test refs for OOPS PR "Hybrid TLM clean up"
4 participants
@kbhargava @travissluka @shlyaeva @tom-j-h