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

UKMO Staging jan2021 #327

Merged
merged 4 commits into from
Mar 19, 2021
Merged

Conversation

ukmo-ccbunney
Copy link
Collaborator

@ukmo-ccbunney ukmo-ccbunney commented Mar 5, 2021

The staging branch comprises two PRs from the ukmo-waves repository:

Fixes #290
Fixes #207
Fixes #314

Full regression test output for each PR are available in the PRs links above.

Regression test outputs for this combined staging branch will be provided below.

Fixes: NOAA-EMC#314
* Interpolation weights now correctly calculated on points next to land and BC locations.
* Changes to improve the code: the possibility of reading zero values from
the input is considered, and points that should not be taken into
account in the interpolation are identified by the netcdf fill value; a
subroutine is created to avoid code duplication
* Fixes NOAA-EMC#290 (ww3_multi hanging when generating restart with IOSTYP >= 2)
* Also fixes out-of-bounds array access error.
* Includes some MPI optimizations
@ukmo-ccbunney ukmo-ccbunney added the bug Something isn't working label Mar 5, 2021
@ukmo-ccbunney ukmo-ccbunney marked this pull request as draft March 5, 2021 12:42
@@ -609,7 +604,7 @@
!/MPI IF ( IP .EQ. NAPRST ) THEN
!/MPI WRITEBUFF(1:NSPEC) = VA(1:NSPEC,JSEA)
!/MPI ELSE
!/MPI JSEA = JSEA - 2*((IB-1)/2)*RSBLKS
!/MPI JSEA = 1 + MOD(IB-1,2)*RSBLKS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do the bug fixes here warrant changing the date for this file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was going to say no, as it as it doesn't actually change the format of the file... but I suppose it could change how an earlier file was read in w.r.t. JSEA? @ukmo-juan-castillo - can you confirm?

@JessicaMeixner-NOAA
Copy link
Collaborator

Does this close any issues? I want to say there is at least one issue this PR would close.

to take into account the coupled
    configurations, that are also affected
@ukmo-ccbunney
Copy link
Collaborator Author

Does this close any issues? I want to say there is at least one issue this PR would close.

@JessicaMeixner-NOAA : yes - I've edited the description to link in the relevant issues.

@ukmo-ccbunney ukmo-ccbunney marked this pull request as ready for review March 17, 2021 12:19
@ukmo-ccbunney
Copy link
Collaborator Author

@ukmo-juan-castillo and I are happy with the regtest results for this PR now. The output of the full regression test maxtrix compared against develop is here:

matrixComp_staging_jan2021.zip

Summary of differences relavent to this PR:

  • mww3_test_04: As expected only the ‘d’ test where we changed the test to write the restart change results
  • ww3_tp2.14: Differences in r-toy.nc and r-ww3.nc are caused OASIS, appending to files rather than overwriting them when a test is re-run; reproducibility is something I want to investigate later with Jessica
  • ww3_tp15: as expected due to corrections in the interpolation of netcdf input fields

Also, the usual culprits which have known differences

  • ww3_tp2.18: Existing known differences in output to screen
  • mww3_test_03: Known to be not b4b.

Over to you @aliabdolali :)

@aliabdolali
Copy link
Contributor

Hi @ukmo-juan-castillo and @ukmo-ccbunney
I am going to check it at our end today.

@aliabdolali
Copy link
Contributor

Hi @ukmo-juan-castillo and @ukmo-ccbunney
The output of the full regression test maxtrix compared against develop is here:
MatrixDiffNCEP.zip
I have similar differences as you mentioned. tp2.14 are identical on our HPC and the ww3_tp2.10 has always differences on our HPC. The rests are the same.

@ukmo-ccbunney
Copy link
Collaborator Author

ukmo-ccbunney commented Mar 18, 2021

Hi @ukmo-juan-castillo and @ukmo-ccbunney
The output of the full regression test maxtrix compared against develop is here:
MatrixDiffNCEP.zip
I have similar differences as you mentioned. tp2.14 are identical on our HPC and the ww3_tp2.10 has always differences on our HPC. The rests are the same.

@aliabdolali I notice that you have some differences in the mod_def.* files for mww3_test_07. I wasn't expecting to see any differences there (and we have not in our tests). Do you have any idea what is happening there (it must be some insignificant difference as all the other output files are identical for that test)?

Everything else looks as expected.

@aliabdolali
Copy link
Contributor

aliabdolali commented Mar 18, 2021

Hi @ukmo-juan-castillo and @ukmo-ccbunney
The output of the full regression test maxtrix compared against develop is here:
MatrixDiffNCEP.zip
I have similar differences as you mentioned. tp2.14 are identical on our HPC and the ww3_tp2.10 has always differences on our HPC. The rests are the same.

@aliabdolali I notice that you have some differences in the mod_def.* files for mww3_test_07. I wasn't expecting to see any differences there (and we have not in our tests). Do you have any idea what is happening there (it must be some insignificant difference as all the other output files are identical for that test)?

Everything else looks as expected.

@ukmo-ccbunney I checked another PR and I had similar non-identical results in mod_def.* files for mww3_test_07. So it is intel/our HPC related.

@ukmo-ccbunney
Copy link
Collaborator Author

ukmo-ccbunney commented Mar 19, 2021

@ukmo-ccbunney I checked another PR and I had similar non-identical results in mod_def.* files for mww3_test_07. So it is intel/our HPC related.

Phew - that's a relief. Also - this is interesting behaviour as sometimes I also have differences in the mod_def.ww3 files for one particular regtest (I forget which one now). I think it might be a compiler issue as the differences usually go away if the filesystem path to the WW3 installation directory has the same number of characters (yes - bizarre I know!!); hence I think it is a GNU Fortran bug/oddity*

* I sometimes forget that compilers are written by humans too!

@JessicaMeixner-NOAA
Copy link
Collaborator

@ukmo-ccbunney I remember we had this happen before because there was an uninitialized variable being stored in the mod_def.ww3 file. I'm not sure if it's the same file/test or not, but something like that could explain why some compilers it's fine and others it is not.

@aliabdolali aliabdolali merged commit 6d95c33 into NOAA-EMC:develop Mar 19, 2021
@ukmo-ccbunney
Copy link
Collaborator Author

Fixes #206

@ukmo-ccbunney ukmo-ccbunney deleted the staging_jan2021 branch May 25, 2021 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants