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 scripts, gnu compiler flags #125

Merged
merged 14 commits into from
Apr 18, 2018
Merged

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Apr 12, 2018

Refactor cice.setup as needed, add setup_run_dirs.csh, add set_version_number.csh, update Macros files for gnu compiler, update -openmp flag for intel compiler, update travis test list, add decomp test list, add decomp options, add decomp test that does multiple runs in a single job, add physics and dynamics options, update version name, remove underflow checks for gnu compiler in debug mode, fix bug in serial halo update, update report_results, add timing information to test results, add multi-suite capability in cice.setup, extend --pes flag in cice.settings to support user defined block sizes.

  • Developer(s): tcraig

  • Are the code changes bit for bit, different at roundoff level, or more substantial? bit-for-bit

  • Is the documentation being updated with this PR? (Y/N) N
    If not, does the documentation need to be updated separately at a later time? (Y/N) Y

  • Other Relevant Details:

Addresses, to some degree #49, #61(?), #63(?), #107, #116, #118, #122.

@apcraig
Copy link
Contributor Author

apcraig commented Apr 12, 2018

This PR is not ready to merge. Doing some travis testing and a few other tasks still TBD.

@apcraig apcraig self-assigned this Apr 12, 2018
@apcraig
Copy link
Contributor Author

apcraig commented Apr 13, 2018

This is probably ready for review and merge. I will run a full test suite on conrad and post results. I am also waiting on travis. NCAR is down for 24 hours or so for annual maintenance, so travis cannot get the input datasets, so I will fire up travis when NCAR is back.

Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

Nice. I ran the quick_suite on pinto, and reported the results, all successfully. Once travis is happy, this looks good to merge.

The one issue, which is not new and does not need to be addressed for this PR (but is somewhat annoying), is that the scripts don't seem to recognize when the jobs have finished. E.g. these "waiting" messages are written for quite a long time after the jobs are actually done:

Submitted batch job 802941
Reporting results
Job 802938 completed
Waiting for 802939 to complete
Waiting for 802939 to complete
Job 802939 completed
Waiting for 802940 to complete
Waiting for 802940 to complete
Waiting for 802940 to complete
Waiting for 802940 to complete
Waiting for 802940 to complete
Job 802940 completed
Job 802941 completed

Could the halo fix correct some other issues like #63 and the Gsum bug in Icepack (which shouldn't occur on physical grounds)?

@apcraig
Copy link
Contributor Author

apcraig commented Apr 14, 2018

The halo fix is in the serial comm directory so would only be used when running without MPI. If #63 is a bug seen in a non-MPI run, this could definitely be the problem. How could we confirm? This also fixes some of the issues we were seeing in travis. I will add the issues of note in the top level summary.

I think I have a couple more things I might try to add to this PR including adding some of the new namelist settings and adding some new test suites for them.

…mp test that does multiple runs in one job. this will serve as a good example for more of that (multiple physics/dynamics options). add several new option settings. add timing information to test results.
@apcraig
Copy link
Contributor Author

apcraig commented Apr 16, 2018

OK, travis is passing. But I also did quite a refactor on the cice.setup script and a few other testing scripts in the last day to support some new features. @eclare108213 @mattdturner, would you like to review again? I will run a full test suite now, post results and update this PR with the results before merging.

@apcraig
Copy link
Contributor Author

apcraig commented Apr 17, 2018

The results of all test suites run on conrad with 4 compilers is here,

https://github.com/CICE-Consortium/Test-Results/wiki/3eed537e29.conrad.pgi.180416.230530
https://github.com/CICE-Consortium/Test-Results/wiki/3eed537e29.conrad.intel.180416.230530
https://github.com/CICE-Consortium/Test-Results/wiki/3eed537e29.conrad.cray.180416.230530
https://github.com/CICE-Consortium/Test-Results/wiki/3eed537e29.conrad.gnu.180416.230530

I don't expect everything to pass, but just shows some additional technical coverage. I am going to put together a suite for some physics/dynamics options with a future PR.

@mattdturner
Copy link
Contributor

It all looks good to me. I should be able to clone and test it tomorrow if necessary, although the test results that you linked show that nothing is broken.

@apcraig apcraig merged commit 5b375c2 into CICE-Consortium:master Apr 18, 2018
@apcraig apcraig mentioned this pull request Jul 10, 2018
@apcraig apcraig deleted the scrA branch August 17, 2022 20:56
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.

None yet

3 participants