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

code cleanup #207

Merged
merged 15 commits into from
Jan 9, 2023
Merged

code cleanup #207

merged 15 commits into from
Jan 9, 2023

Conversation

jedwards4b
Copy link
Contributor

@jedwards4b jedwards4b commented Jan 3, 2023

Description of changes

Remove all gfortran warnings except unused-dummy-argument with the exception of the external fox code.

Specific notes

Contributors other than yourself, if any:

CDEPS Issues Fixed #204

Are there dependencies on other component PRs (if so list):

Are changes expected to change answers (bfb, different to roundoff, more substantial): bfb

Any User Interface Changes (namelist or namelist defaults changes): no

Testing performed (e.g. aux_cdeps, CESM prealpha, etc): cesm prealphas on cheyenne with intel and gnu and updated github testing.

Hashes used for testing: cesm2.3.beta11 + cmeps 2ded3b5
cdeps 7bd2b80

@jedwards4b jedwards4b requested a review from mvertens January 3, 2023 22:44
@jedwards4b jedwards4b requested a review from uturuncoglu January 4, 2023 15:33
@jedwards4b jedwards4b self-assigned this Jan 4, 2023
@jedwards4b
Copy link
Contributor Author

@mvertens @uturuncoglu I have added the -Werror flag to the github tests for directories excluding fox.
This will cause any gnu compiler warning in cdeps to generate an error in the github testing.

@jedwards4b
Copy link
Contributor Author

I should say that I did add one exception: -Wno-unused-dummy-argument because the callbacks from esmf sometimes include import and export State variables that are not used.

@jedwards4b jedwards4b mentioned this pull request Jan 4, 2023
9 tasks
@jedwards4b
Copy link
Contributor Author

@uturuncoglu can you review this please?

@mvertens
Copy link
Collaborator

mvertens commented Jan 6, 2023

@jedwards4b - thanks so much for doing this. Can we arrange a time to chat to go over your changes in more detail. I have several questions that would be resolved much more quickly in a chat.

@jedwards4b
Copy link
Contributor Author

@mvertens sure.

Copy link
Collaborator

@uturuncoglu uturuncoglu left a comment

Choose a reason for hiding this comment

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

@jedwards4b I'll test this under UFS. I also need to test the standalone build to be sure that it is not breaking anything in datm+land GitHub Action in NoahMP side. That could take little bit time but I'll try to finish them today.

.github/workflows/extbuild.yml Show resolved Hide resolved
@uturuncoglu
Copy link
Collaborator

@jedwards4b i tested under UFS by running couple of datm RTs incl. restart and all are passed. So, there is no issue under UFS. I'll try to test standalone build too and let you know.

@uturuncoglu
Copy link
Collaborator

@jedwards4b I am not sure why but I am getting error under Docker container that uses Ubuntu and following fortran version

gcc version 11.3.0 (Ubuntu 11.3.0-1ubuntu1~22.04)

the error is

/test/CDEPS/share/shr_infnan_mod.F90.in:73:20:

   73 | #endif
      |                    1
Error: Procedure 'shr_infnan_isnan_real' in generic interface 'shr_infnan_isnan' at (1) is neither function nor subroutine; did you mean 'shr_infnan_isinf_real'?
make[2]: *** [share/CMakeFiles/cdeps_share.dir/build.make:87: share/CMakeFiles/cdeps_share.dir/shr_infnan_mod.F90.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:395: share/CMakeFiles/cdeps_share.dir/all] Error 2

maybe this is not related with this PR but I am not getting this error from my fork

https://github.com/uturuncoglu/CDEPS/tree/hotfix/std_build

@jedwards4b
Copy link
Contributor Author

Is -DCPRGNU defined in your test build?

@uturuncoglu
Copy link
Collaborator

@jedwards4b No. how can I define it? If I give it like -DCPRGNU=ON to cmake command it complains. BTW, I am trying to update my branch with CDEPS/master and plan to test again. If it works, I'll merge with your fork as a next step to test it.

@jedwards4b
Copy link
Contributor Author

I think if you set CPPDEFS="-DCPRGNU" in the environment before invoking cmake

@jedwards4b
Copy link
Contributor Author

Or as in the github workflow here.

@uturuncoglu
Copy link
Collaborator

@jedwards4b Okay. The GitHub way worked. Thanks. I could able to compile it now.

Probably I'll open another PR later to fix the FoX library issue because the standalone build is forcing FoX and if you would like to use ESMF Config format for stream namelist it is not possible. I have a fix in my fork that enables it by bringing -DDISABLE_FoX=ON option.

@jedwards4b jedwards4b merged commit 2796c71 into ESCOMP:master Jan 9, 2023
@jedwards4b jedwards4b deleted the more_github_workflow branch January 9, 2023 17:26
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.

question about case_name in dice
3 participants