Skip to content

2878 extraction update#2907

Merged
sergisiso merged 21 commits into
masterfrom
2878_extraction_update
Jul 29, 2025
Merged

2878 extraction update#2907
sergisiso merged 21 commits into
masterfrom
2878_extraction_update

Conversation

@hiker

@hiker hiker commented Feb 25, 2025

Copy link
Copy Markdown
Collaborator

Fixed #2878 (and other minor improvements to output format to make it easier to read).

@codecov

codecov Bot commented Feb 25, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.90%. Comparing base (08ee5ba) to head (6b8620e).
⚠️ Report is 22 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2907   +/-   ##
=======================================
  Coverage   99.90%   99.90%           
=======================================
  Files         368      368           
  Lines       51992    51992           
=======================================
  Hits        51942    51942           
  Misses         50       50           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hiker

hiker commented Jul 4, 2025

Copy link
Copy Markdown
Collaborator Author

This makes the extraction comparison more readable (using ints instead of floating point format), fixes a but or two, and also includes the a compiled and executed test of the comparison library.

@sergisiso sergisiso left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@hiker Just some minor comments, I like the new extraction stats table, it's easier to quickly see if there is important differences imo.

Comment thread .github/workflows/compilation.yml Outdated
Comment thread doc/user_guide/psyke.rst
Comment thread lib/extract/Makefile Outdated
Comment thread lib/extract/Makefile Outdated
Comment thread lib/extract/netcdf/lfric/Makefile
Comment thread lib/extract/test_compare.f90 Outdated
Comment thread lib/extract/Makefile
@hiker hiker temporarily deployed to integration July 8, 2025 03:48 — with GitHub Actions Inactive
@hiker

hiker commented Jul 25, 2025

Copy link
Copy Markdown
Collaborator Author

Looks like I forgot to mark this as ready. I've brought it up to latest master, and triggered the CI (which should not really be affected by the changes here).

@sergisiso sergisiso left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@hiker All comments have been addressed, this is ready to merge if the integration tests come green.

@sergisiso

Copy link
Copy Markdown
Collaborator

@hiker Unfortunately the libraries compilation failed with:

./test_compare | grep "a_dbl    *15 *1 *2 *3 *4 *5" > /dev/null
At line 147 of file compare_variables_mod.F90 (unit = 6, file = 'stdout')
Fortran runtime error: Missing comma between descriptors
(A8' ',6(I12, ' '),4(E12.7,' '))                                                
     ^

@hiker

hiker commented Jul 28, 2025

Copy link
Copy Markdown
Collaborator Author

Which compiler (and version) is that? I've tried nvfortran (23.5), gfortran (11.4), ifort (2021.8.0), and ifx(2023.0.0), and they are all working fine.

Ah, found it - gfortran 15. As far as I can see, I don't have access to that atm :( I'll see if I can figure out what is happening.

@hiker

hiker commented Jul 28, 2025

Copy link
Copy Markdown
Collaborator Author

I think I got it. I'll trigger the full CI tomorrow.

@hiker

hiker commented Jul 29, 2025

Copy link
Copy Markdown
Collaborator Author

I've fixed the conflict in the changelog, I don't think there's a need to rerun the CI because of this.

@sergisiso

Copy link
Copy Markdown
Collaborator

Thanks @hiker, this ca be merged now

@sergisiso sergisiso merged commit a320ebd into master Jul 29, 2025
11 checks passed
@sergisiso sergisiso deleted the 2878_extraction_update branch July 29, 2025 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[PSyData] Minor extraction library updates

2 participants