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

Update ecwam regression tests to use develop branch #335

Merged
merged 5 commits into from
Jul 26, 2024

Conversation

reuterbal
Copy link
Collaborator

No description provided.

Copy link

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/335/index.html

Copy link

codecov bot commented Jun 26, 2024

Codecov Report

Attention: Patch coverage is 10.71429% with 25 lines in your changes missing coverage. Please review.

Project coverage is 95.31%. Comparing base (7ca9dd8) to head (97dad78).
Report is 5 commits behind head on main.

Files Patch % Lines
loki/transformations/tests/test_ecwam.py 10.71% 25 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #335   +/-   ##
=======================================
  Coverage   95.31%   95.31%           
=======================================
  Files         170      171    +1     
  Lines       36467    36548   +81     
=======================================
+ Hits        34759    34837   +78     
- Misses       1708     1711    +3     
Flag Coverage Δ
lint_rules 96.39% <ø> (ø)
loki 95.30% <10.71%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@reuterbal reuterbal force-pushed the nabr-update-ecwam-regression branch 2 times, most recently from 111a88b to 1a6baca Compare June 26, 2024 19:03
@reuterbal reuterbal force-pushed the nabr-update-ecwam-regression branch from 32d22b3 to 172bee8 Compare July 22, 2024 13:50
@reuterbal reuterbal force-pushed the nabr-update-ecwam-regression branch from 172bee8 to 8945845 Compare July 24, 2024 07:21
awnawab and others added 2 commits July 26, 2024 07:06
@reuterbal
Copy link
Collaborator Author

Thanks to @awnawab this should be ready now!

Copy link
Collaborator

@mlange05 mlange05 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 to me, and indeed many thanks for figuring out all the nifty details @reuterbal and @awnawab . I left one small comment on the use of the default arch (is this intentional and safe?), but if someone can confirm that this is fine, we're GTG. 👍 :shipit:


- name: Run CLOUDSC and ECWAM regression tests
env:
CLOUDSC_DIR: ${{ github.workspace }}/cloudsc
CLOUDSC_ARCH: ${{ github.workspace }}/cloudsc/arch/github/ubuntu/gnu/9.4.0
ECWAM_DIR: ${{ github.workspace }}/ecwam
ECWAM_ARCH: ${{ github.workspace }}/ecwam/arch/github/ubuntu/gnu/9.4.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just double-checking: Is relying on default arch safe here? @awnawab

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My understanding is that implies no arch files are used, i.e., no specific toolchain.cmake and env.sh. Since none are required, this should be fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes exactly, the ecwam CI also runs without any arch files and just relies on the default cmake/ecbuild flags plus those set by ecwam_compile_flags.cmake.

@mlange05 mlange05 added the ready for merge This PR has been approved and is ready to be merged label Jul 26, 2024
Copy link
Contributor

@awnawab awnawab left a comment

Choose a reason for hiding this comment

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

Many thanks for this, apologies for not wrapping this up sooner myself, it was long overdue. I can now finally forget about that very old ecwam dev branch that we were previously using 😄

@reuterbal reuterbal merged commit bac8b3a into main Jul 26, 2024
12 checks passed
@reuterbal reuterbal deleted the nabr-update-ecwam-regression branch July 26, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for merge This PR has been approved and is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants