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 value for checks%range #345

Merged
merged 4 commits into from
Jul 16, 2024
Merged

Conversation

ccarouge
Copy link
Collaborator

@ccarouge ccarouge commented Jul 15, 2024

CABLE

Thank you for submitting a pull request to the CABLE Project.

Description

Fixes #344. Update the namelist provided with the code for the changes to check%range and other fixes for compatibility with GNU compiler.

Type of change

Please delete options that are not relevant.

  • Bug fix

Checklist

I have compiled the code and run the Tumbarumba test case with the updated namelist. The test case finished:

 Finished.    36.78621      seconds needed for year
 Finished.    36.78769      seconds needed for        70128  hours

Please add a reviewer when ready for review.


📚 Documentation preview 📚: https://cable--345.org.readthedocs.build/en/345/

@ccarouge
Copy link
Collaborator Author

@abhaasgoyal we forgot to update this namelist file with the changes to check the range.

Copy link
Collaborator

@har917 har917 left a comment

Choose a reason for hiding this comment

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

@ccarouge @abhaasgoyal This change looks okay ... BUT ...

I'm not sure about previous change - we've switched from a TRUE/FALSE LOGICAL kind for check%ranges to an integer but there's still a few instances in the code of IF(check%range)THEN

I can see that working for check%range=0,1 but not for 2 - at least with some compilers.

Copy link
Collaborator

@har917 har917 left a comment

Choose a reason for hiding this comment

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

In testing #335 and #346 I tested with check%ranges = 0, 1, 2. This would seem to be fine - at least for the compiler on gadi.

@ccarouge
Copy link
Collaborator Author

@har917 The main branch does not currently compile with GNU compiler anyway for other reasons (the abort() function for example accepts a string argument with Intel but not GNU...). But I agree we should change the IF statements as we don't want to go further from compatibility with GNU than we currently are. It's just going to be tricky to see all of these without being able to compile. I'll add that change and update the issue.

@abhaasgoyal
Copy link
Contributor

abhaasgoyal commented Jul 15, 2024

@ccarouge For clarity, maybe we can have the check check%ranges /= NO_CHECK in the IF statement?

@har917 wanted to ask for which compilers is check%range=2 failing for? Edit: I see it is with the GNU compiler

Copy link
Contributor

@abhaasgoyal abhaasgoyal left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@abhaasgoyal abhaasgoyal left a comment

Choose a reason for hiding this comment

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

Edit: Apologies since already approved, but there was a small change which if done would be good. There are some check%ranges=.TRUE. in the documentation, which can also be changed (in pft_params_nml.md, and meteorological_forcings.md).

Copy link
Contributor

@abhaasgoyal abhaasgoyal left a comment

Choose a reason for hiding this comment

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

LGTM

@ccarouge ccarouge merged commit c2a54da into main Jul 16, 2024
7 checks passed
@ccarouge ccarouge deleted the 344-update-type-rangechecks-cablenml branch July 16, 2024 05:21
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.

Fix namelist incompatibility and some incompatibilities for GNU compiler in the new checks%range
3 participants