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

separate model variables and obs variables types #70

Merged
merged 2 commits into from
May 20, 2024

Conversation

twsearle
Copy link
Collaborator

@twsearle twsearle commented May 20, 2024

Description

Use the new oops::ObsVariables for observations and oops::Variables for model variables.

Issue(s) addressed

Resolves #69

Impact

Resolves compile-time errors due to recent oops changes.

Checklist

  • I have updated the unit tests to cover the change
  • I have linted my code using cpplint
  • I have run the unit tests
  • I have run mo-bundle to check integration with the rest of JEDI and run the unit tests under all environments

@twsearle twsearle self-assigned this May 20, 2024
@twsearle twsearle linked an issue May 20, 2024 that may be closed by this pull request
@twsearle twsearle marked this pull request as ready for review May 20, 2024 13:08
@twsearle
Copy link
Collaborator Author

I am seeing a lot of mo-bundle issues that don't seem to be related to my change, but might be related to me commenting out all lfric etc libraries:
http://fcm1/cylc-review/view/tsearle?&suite=mobb-oops2591&no_fuzzy_time=0&path=log/job/1/cmake_mo__cray_gnu_debug/01/job.err

changes look straight-forward to me otherwise and unit tests pass. If I get a green light I will push ahead as we really want to fix this up as quickly as possible to be able to run all the comparison tests etc.

Copy link
Collaborator

@mo-joshuacolclough mo-joshuacolclough left a comment

Choose a reason for hiding this comment

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

Thanks looks OK to me, although not an expert on this repo, and as mentioned on orca-jedi I haven't run the tests.

@twsearle
Copy link
Collaborator Author

Thanks looks OK to me, although not an expert on this repo, and as mentioned on orca-jedi I haven't run the tests.

The tests are run as part of the CI, so no need if there is a green tick :) Its more the full integration and build on XC and spice that concerned me, but I think that is probably just me commenting out too many repos. As you say, it can't really get worse and I can always open another if there are still issues.

@twsearle twsearle merged commit 002f15a into develop May 20, 2024
2 checks passed
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.

Clearly separate obs-world variables from model-world variables
2 participants