COSIMA / datetime-fortran

Date and time manipulation for modern Fortran
https://wavebitscientific.github.io/datetime-fortran/
Other
0 stars 0 forks source link

Use reproducibility flags for Gadi #1

Open penguian opened 4 years ago

penguian commented 4 years ago

Document the reproducibility flags that were used in testing https://github.com/penguian/1deg_jra55_ryf migration from NCI Raijin to Gadi.

Using these flags with the Intel 2019.3.199 compilers produced identical output on Raijin and Gadi. See "repro group 4" in the report summary for results.

aekiss commented 4 years ago

sorry @nichannah I should have let you merge https://github.com/COSIMA/datetime-fortran/commit/79b597b891c7a636b9cdf0d1c264fa6b14332cb6 - I think this might have broken things.

I gather from that the point of this 51-avoid-date2num-compiler-bug branch is that CMAKE_Fortran_FLAGS_RELEASE should use -O1, not -O2, to avoid a "weird compiler bug", so line 27 of CMakeLists.txt presumably needs fixing now. (before this merge, this branch used -O1, whereas master uses O3.)

aekiss commented 4 years ago

I've made a PR to fix this. The unit test should detect whether it is needed (but note the test icon in README is for https://travis-ci.org/wavebitscientific/datetime-fortran, not this branch of this COSIMA fork).

penguian commented 4 years ago

It would help to have a reference to the compiler bug. The reference in datetime-fortran is: https://github.com/wavebitscientific/datetime-fortran/issues/51

Also, I believe that I have found a bug in num2date: https://github.com/COSIMA/datetime-fortran/blob/51-avoid-date2num-compiler-bug/src/lib/mod_datetime.f90 line 1245:

  if(num2date % hour == 60)then

Surely that comparison should be to 24, not 60? I will create a separate issue.

aekiss commented 4 years ago

I guess we don't know yet whether the https://github.com/wavebitscientific/datetime-fortran/issues/51 bug occurs with the 2019 compiler, so PR https://github.com/COSIMA/datetime-fortran/pull/3 might not be necessary.

nichannah commented 4 years ago

Compiler flags should make no difference to the results from this library. If they did it would indicate a bug.

I think, if possible, it would be good if we can stick with the upstream defaults. The will reduce the amount of maintenance at our end.

Also, perhaps until we have project or platform specific changes it would be good if we could use upstream unchanged, i.e. no need for our own fork.

penguian commented 4 years ago

Closing, as pull request #2 was merged and closed.

Does this need another issue to revert to upstream defaults?

aekiss commented 4 years ago

oops, I merged this branch into master (when I intended to do the opposite), after updating the master from upstream. Sorry, I should have read this thread more carefully.

Changes from master are small (compiler flags and a couple of tests): https://github.com/COSIMA/datetime-fortran/compare/914d21e4..f3783177d9ef

@nichannah will this be a pain for keeping up to date with upstream?

aekiss commented 4 years ago

probably the simplest fix is to trash this fork and re-fork directly from https://github.com/wavebitscientific/datetime-fortran