Adding method to check namelist in any order, tested with NAG Fortran. #801

daveh150 · 2022-12-02T18:40:44Z

PR checklist

Short (1 sentence) summary of your PR:
Check namelist in any order to support NAG compiler.
Developer(s):
David Hebert
Suggest PR reviewers from list in the column to the right.
Please copy the PR test results link or provide a summary of testing completed below.
ran quicksuite, modified for tests to run with 1 PE. Compiled main branch with gnu for baseline.
Then compiled check_namelist2 branch with gnu and nag compilers. Results show all tests past.
Also added errors to namelists. Code crashes with message on what line has error.:

(abort_ice)ABORTED:
(abort_ice) called from ice_init.F90
(abort_ice) line number 644
(abort_ice) error = (input_data)ERROR: grid_nml reading test_namelist = .true.

Re-opening #788. Implemented method to rewind and search ice_in for namelist, then read namelist and check for errors. This is done through addition of subroutine goto_nml_group in ice_namelist_mod.F90. I added new file because of circular dependencies between ice_in.F90 and ice_domain.F90.

I have a trial NAG FORTRAN compiler license for a few more days, this has been tested with serial NAG and GNU compiler. I inserted error in namelists and run crashes with message like:

(abort_ice)ABORTED:
(abort_ice) called from ice_init.F90
(abort_ice) line number 644
(abort_ice) error = (input_data)ERROR: grid_nml reading test_namelist = .true.

I also reordered the namelists, putting grid_nml before setup_nml, and both compilers worked.

I admittedly do not have mpi fortran compiled with NAG, but since this is only a namelist check I am hoping mpi vs. serial won't matter. If someone has access to machine with NAG FORTRAN MPI to do a more thorough test that would be helpful.

Disclosure, after much googling for namelist, method is based on this link:
https://stackoverflow.com/questions/57553995/how-to-handle-an-optional-group-in-a-fortran-namelist

…amelist_mod.F90 to search for namelist in ice_in.

apcraig · 2022-12-02T18:57:37Z

Thanks @daveh150 for continuing to pursue this! This looks great assuming it's robust. I can run some tests on more compilers/platforms (although I don't have access to nag at the moment either). One comment, rather than create a new file for the new subroutine, how about dropping it into cicecore/shared/ice_fileunits.F90. I think it's both a reasonable place and shouldn't have any problems with circular logic. Thoughts?

daveh150 · 2022-12-02T19:30:45Z

Thanks @apcraig! Sure, moving the goto_nml_group to ice_fileunits.F90 seems the best location without adding more files. I'll make that move now.

…mod.F90

daveh150 · 2022-12-02T20:24:52Z

I moved the goto_ nml subroutine to ice_fileunits.F90. If there are any other thoughts or suggestions please let me know.

apcraig · 2022-12-05T18:53:56Z

FYI, I plan to defer this PR until next week, after the release. I want to be able to adequately test on several platforms and make sure everything is robust. We've had issue in the past, although I think this solution should be fine. Just don't want to introduce unnecessary risk this late. Thanks.

daveh150 · 2022-12-05T18:56:47Z

Sure, no problem. I agree it should be tested further than what I was able to do here. Especially if there is a machine with NAG compiler, my trial version ends this week. From: Tony Craig ***@***.***> Sent: Monday, December 5, 2022 12:54 To: CICE-Consortium/CICE ***@***.***> Cc: David Hebert, Code 7322 ***@***.***>; Mention ***@***.***> Subject: Re: [CICE-Consortium/CICE] Adding method to check namelist in any order, tested with NAG Fortran. (PR #801) FYI, I plan to defer this PR until next week, after the release. I want to be able to adequately test on several platforms and make sure everything is robust. We've had issue in the past, although I think this solution should be fine. Just don't want to introduce unnecessary risk this late. Thanks. — Reply to this email directly, view it on GitHub <#801 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE52VPAPQSMGP7HMS2Z4AWDWLY247ANCNFSM6AAAAAASSGFSXM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

apcraig · 2022-12-15T23:46:58Z

cicecore/cicedyn/analysis/ice_history_bgc.F90

@@ -305,24 +309,41 @@ subroutine init_hist_bgc_2D
      !-----------------------------------------------------------------

      if (my_task == master_task) then
-         write(nu_diag,*) subname,' Reading icefields_bgc_nml'
+        nml_name = 'icefields_bgc_nml'


Looks like this line needs an extra space for alignment.

cicecore/cicedyn/analysis/ice_history_fsd.F90

apcraig · 2022-12-15T23:50:10Z

cicecore/cicedyn/general/ice_init.F90

@@ -165,6 +167,7 @@ subroutine input_data
      integer (kind=int_kind) :: rplvl, rptopo
      real (kind=dbl_kind) :: Cf, ksno, puny
      character (len=char_len) :: abort_list
+      character(len=char_len) :: nml_name ! namelist name


alignment with line above/below?

apcraig · 2022-12-15T23:51:01Z

cicecore/cicedyn/general/ice_init.F90

+
+               call abort_ice(subname//'ERROR: '//trim(nml_name)//' reading '// &
+                    trim(tmpstr2), &
+                    file=__FILE__, line=__LINE__)


combine onto prior line?

I found the line to be too long with the entire concatenated string on a single line. I prefer to leave it as it, but if others find it too confusing I can put it all on one line.

I might combine the last two lines, tmpstr2 and file/line, leaving first line alone?

Okay, I see now. I combined the last to to read trim (tmpstr2), file=...

I think I have all other intents/spacings updated. Let me know if I missed anything. Thanks!

apcraig · 2022-12-15T23:51:18Z

cicecore/cicedyn/general/ice_init.F90

+               read(nu_nml,fmt='(A)') tmpstr2
+               call abort_ice(subname//'ERROR: '// trim(nml_name)//' reading '// &
+                    trim(tmpstr2), &
+                    file=__FILE__, line=__LINE__)


same, check other instances too

cicecore/cicedyn/infrastructure/ice_domain.F90

apcraig · 2022-12-16T16:42:04Z

Testing seems to be good. I think we can merge this. Maybe one more quick pass looking at the indentation things noted above, then we can merge. Thanks.

daveh150 · 2022-12-16T16:43:41Z

Great, thanks Tony! I’ll clean up the indentation now. From: Tony Craig ***@***.***> Sent: Friday, December 16, 2022 10:42 To: CICE-Consortium/CICE ***@***.***> Cc: David Hebert, Code 7322 ***@***.***>; Mention ***@***.***> Subject: Re: [CICE-Consortium/CICE] Adding method to check namelist in any order, tested with NAG Fortran. (PR #801) Testing seems to be good. I think we can merge this. Maybe one more quick pass looking at the indentation things noted above, then we can merge. Thanks. — Reply to this email directly, view it on GitHub <#801 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE52VPHW7VBHSJOBQ7Q2GO3WNSLWNANCNFSM6AAAAAASSGFSXM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

…string.

Adding method to check namelist in any order. Use subroutine in ice_n…

a00edf3

…amelist_mod.F90 to search for namelist in ice_in.

daveh150 added the Software Engineering label Dec 2, 2022

Moved goto_nml subroutine to ice_fileunits.F90. Removed ice_namelist_…

3330b9f

…mod.F90

apcraig reviewed Dec 15, 2022

View reviewed changes

cicecore/cicedyn/analysis/ice_history_fsd.F90 Show resolved Hide resolved

apcraig reviewed Dec 15, 2022

View reviewed changes

cicecore/cicedyn/infrastructure/ice_domain.F90 Show resolved Hide resolved

daveh150 and others added 4 commits December 16, 2022 11:08

Cleanup indentations with tmpstr2 use

542b8d3

Cleanup spacing and intentation

cf9d1e5

For namelist check, remove extra continuation after making ice_abort …

7d97187

…string.

Merge branch 'main' into check_namelist2

d456ba3

apcraig approved these changes Dec 16, 2022

View reviewed changes

apcraig merged commit eebb350 into CICE-Consortium:main Dec 16, 2022

DeniseWorthen mentioned this pull request Mar 22, 2023

updates from Consortium/main NOAA-EMC/CICE#58

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding method to check namelist in any order, tested with NAG Fortran. #801

Adding method to check namelist in any order, tested with NAG Fortran. #801

daveh150 commented Dec 2, 2022

apcraig commented Dec 2, 2022

daveh150 commented Dec 2, 2022

daveh150 commented Dec 2, 2022

apcraig commented Dec 5, 2022

daveh150 commented Dec 5, 2022 via email

apcraig Dec 15, 2022

apcraig Dec 15, 2022

apcraig Dec 15, 2022

daveh150 Dec 16, 2022

apcraig Dec 16, 2022

daveh150 Dec 16, 2022

apcraig Dec 15, 2022

apcraig commented Dec 16, 2022

daveh150 commented Dec 16, 2022 via email

Adding method to check namelist in any order, tested with NAG Fortran. #801

Adding method to check namelist in any order, tested with NAG Fortran. #801

Conversation

daveh150 commented Dec 2, 2022

PR checklist

apcraig commented Dec 2, 2022

daveh150 commented Dec 2, 2022

daveh150 commented Dec 2, 2022

apcraig commented Dec 5, 2022

daveh150 commented Dec 5, 2022 via email

apcraig Dec 15, 2022

Choose a reason for hiding this comment

apcraig Dec 15, 2022

Choose a reason for hiding this comment

apcraig Dec 15, 2022

Choose a reason for hiding this comment

daveh150 Dec 16, 2022

Choose a reason for hiding this comment

apcraig Dec 16, 2022

Choose a reason for hiding this comment

daveh150 Dec 16, 2022

Choose a reason for hiding this comment

apcraig Dec 15, 2022

Choose a reason for hiding this comment

apcraig commented Dec 16, 2022

daveh150 commented Dec 16, 2022 via email