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

updates to use either PR1 or PR3 easily #9

Merged
merged 5 commits into from
Jul 14, 2023

Conversation

mvertens
Copy link
Collaborator

@mvertens mvertens commented Jul 14, 2023

Pull Request Summary

Update to use either PR1 or PR3 easily

Description

This PR updates CMakeLists.txt to use PR1 and PR3 easily (enhancement)
Also - provides a bugfix for running PR1 in debug mode (bug)

Issue(s) addressed

Commit Message

updates and bug fixes to use either PR1 or PR3 easily

Testing

Ran the following tests:
SMS_D_Ld5.T62_wtn14nw.WW3test.betzy_intel
ERS_D_Ld5.T62_wtn14.WW3test.betzy_intel

@mvertens mvertens added bug fix Something isn't working enhancement New feature or request labels Jul 14, 2023
@mvertens mvertens requested a review from gold2718 July 14, 2023 12:54
@mvertens
Copy link
Collaborator Author

@gold2718 - if you could please review this - I'll then make a ww3dev PR that will point to this tag.

Copy link

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

Approved but with some reservation about more #ifdef code than is necessary.

Comment on lines 261 to 265
#ifndef W3_CESMCOUPLED
inquire(file=trim(fnmpre)//"ww3_shel.nml", exist=flgnml)
#else
inquire(file=trim(fnmpre)//"wav_in", exist=flgnml)
#endif

Choose a reason for hiding this comment

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

It would be nice if the filename was resolved in one place (into a local variable) and then used here and below. That way, the code difference between models is minimized.
-- Committee for less ifdef code

Suggested change
#ifndef W3_CESMCOUPLED
inquire(file=trim(fnmpre)//"ww3_shel.nml", exist=flgnml)
#else
inquire(file=trim(fnmpre)//"wav_in", exist=flgnml)
#endif
#ifndef W3_CESMCOUPLED
nl_filename = trim(fnmpre)//"ww3_shel.nml"
#else
nl_filename = trim(fnmpre)//"wav_in"
#endif
inquire(file=trim(nl_filename), exist=flgnml)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gold2718 - I agree. I had done that originally and had other problems associated with changing the namelist.
I'll implement your suggestion now.

@mvertens mvertens merged commit 65942b6 into NorESMhub:noresm Jul 14, 2023
0 of 2 checks passed
@mvertens mvertens deleted the feature/updated_workflow branch July 14, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants