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

Formatting fix for the write to obs_seq.final with cce #491

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mjs2369
Copy link
Contributor

@mjs2369 mjs2369 commented Jun 7, 2023

Description:

There are inconsistencies in the obs_seq.final formatting across compilers on Gust. Specifically, the cce compiler produces an output of 2*-1 rather than -1 -1 (for intel and ifx) for obs%prev_time, obs%next_time, and obs%cov_group

I added formatting to the write statement on line 2508 of obs_sequence_mod.f90

Fixes issue

Fixes #484

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Documentation changes needed?

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.

Tests

Ran filter with lorenz_96 on Gust with intel and cce, compared obs_seq.final files.

Checklist for merging

  • Updated changelog entry
  • Documentation updated
  • Update conf.py

Checklist for release

  • Merge into main
  • Create release from the main branch with appropriate tag
  • Delete feature-branch

Testing Datasets

  • Dataset needed for testing available upon request
  • Dataset download instructions included
  • No dataset needed

@mjs2369 mjs2369 requested a review from hkershaw-brown June 7, 2023 21:47
@mjs2369
Copy link
Contributor Author

mjs2369 commented Jun 7, 2023

@hkershaw-brown there is still a bit of difference in these lines of the obs_seq.final. The first is with Intel, the second with cce after the formatting was added:

       39999          -1          -1
   39999,      -1,      -1

I added commas to the format statement because they are present in the obs_seq.final originally with cce. But they aren't with Intel. Similarly, there's extra spaces between values with intel.

Do you think we should make them look identical in these ways as well?

@mjs2369
Copy link
Contributor Author

mjs2369 commented Jun 8, 2023

With edits made, the writes for obs%prev_time, obs%next_time, and obs%cov_group output as such for the different compilers.

intel:
       39999          -1          -1
gfortran:
       39999          -1          -1
cce:
   39999      -1      -1

cce is just missing some spacing.

@mjs2369
Copy link
Contributor Author

mjs2369 commented Jun 8, 2023

Final edits fix the inconsistencies in the spacing so that all compilers produce the exact same output. This also matches the formatting for intel and gfortran on Cheyenne.

intel:
       39999          -1          -1
gfortran:
       39999          -1          -1
cce:
       39999          -1          -1

@mjs2369 mjs2369 added the release! bundle with next release label Jun 12, 2023
@hkershaw-brown hkershaw-brown removed the release! bundle with next release label Jun 20, 2023
@mjs2369 mjs2369 mentioned this pull request Sep 26, 2023
15 tasks
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.

bug: obs_seq.final formatting inconsistencies across compilers on Gust
2 participants