Skip to content

Feature/tp2.14#607

Merged
JessicaMeixner-NOAA merged 12 commits into
NOAA-EMC:developfrom
umr-lops:feature/tp2.14
Feb 17, 2022
Merged

Feature/tp2.14#607
JessicaMeixner-NOAA merged 12 commits into
NOAA-EMC:developfrom
umr-lops:feature/tp2.14

Conversation

@mickaelaccensi
Copy link
Copy Markdown
Collaborator

@mickaelaccensi mickaelaccensi commented Feb 10, 2022

Pull Request Summary

reorganize ww3_tp2.14 by input subdirectories to avoid files renaming inside input dir (different cases if inp or nml)
update ww3_from_ftp.sh and matrix.base to have ww3_tp2.14 working with namelist
minor update in the coupling itself to use the same conditions for each exchanged variable.

Description

input directories are now splitted with their own input files

info   input_oasacm   input_oasacm3  input_oasacm5  input_oasicm
input  input_oasacm2  input_oasacm4  input_oasacm6  input_oasocm

toy model and oasis3-mct are compiled in each work directory to not modify the input directories

Issue(s) addressed

fixes #513

Commit Message

reorganize ww3_tp2.14 by separated input directories and fix it to use with namelist files
minor update in the coupling itself to use the same conditions for each exchanged variable.

Check list

Testing

  • How were these changes tested?
    regtest ww3_tp2.14
  • Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
    regtest ww3_tp2.14
  • Have the matrix regression tests been run (if yes, please note HPC and compiler)?
    only regtest ww3_tp2.14 on datarmor HPC with intel compiler
  • Please provide the summary output of matrix.comp (matrix.Diff.txt, matrixCompFull.txt and matrixCompSummary.txt):
    matrixDiff.txt
    matrixCompSummary.txt
    matrixCompFull.txt
  • Please indicate the expected changes in the regression test output (Note the known list of non-identical tests).
    input path changes in log files

@mickaelaccensi mickaelaccensi self-assigned this Feb 10, 2022
@mickaelaccensi mickaelaccensi added the Regression Test A regression test is needed for a feature label Feb 10, 2022
@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

So the changes in model/src/w3ogcmmd.F90 are related to namelist?

@mickaelaccensi
Copy link
Copy Markdown
Collaborator Author

So the changes in model/src/w3ogcmmd.F90 are related to namelist?

it is a minor update in the coupling itself to use the same conditions for each exchanged variable.

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

Can you add that to the PR and commit message? I've asked @ukmo-ccbunney to review as well since the changes are related to OASIS. I've started the regression tests on my end.

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

@mickaelaccensi I saw you pushed an update to this, will this address the diffs I'm getting in the 2.14 regtests:
ww3_tp2.14/./work_OASACM4 (3 files differ)
ww3_tp2.14/./work_OASICM (2 files differ)
ww3_tp2.14/./work_OASACM3 (2 files differ)
ww3_tp2.14/./work_OASACM6 (6 files differ)
ww3_tp2.14/./work_OASOCM (3 files differ)
ww3_tp2.14/./work_OASACM2 (3 files differ)
ww3_tp2.14/./work_OASACM5 (3 files differ)
ww3_tp2.14/./work_OASACM (3 files differ)

Or are these expected differences?

@mickaelaccensi
Copy link
Copy Markdown
Collaborator Author

@mickaelaccensi I saw you pushed an update to this, will this address the diffs I'm getting in the 2.14 regtests: ww3_tp2.14/./work_OASACM4 (3 files differ) ww3_tp2.14/./work_OASICM (2 files differ) ww3_tp2.14/./work_OASACM3 (2 files differ) ww3_tp2.14/./work_OASACM6 (6 files differ) ww3_tp2.14/./work_OASOCM (3 files differ) ww3_tp2.14/./work_OASACM2 (3 files differ) ww3_tp2.14/./work_OASACM5 (3 files differ) ww3_tp2.14/./work_OASACM (3 files differ)

Or are these expected differences?

no it won't change anything if you run with inp files. It's only for nml files.
Your differences are expected, it's only related to the changes of input path and warning messages removed

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

Perfect, thanks for the explanation! I see that it's mostly just output files now and a lot of "not in compare" which I think is related to the updates you have made with regards to some of the compiling for oasis. Question though, it seems like we are comparing some files that maybe we don't want to, for example:
debug.root.01 differ.
toy_model differ (binary)
namcouple differ.

I'm going to run the ww3_tp2.14 tests from this branch again and compare the two to make sure that as we move forward we don't have any unexpected diffs that we're looking out for.

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

I am setting up the second ww3_tp2.14 tests and noticed that I don't think we need the:
"mkdir -p ww3_tp2.14/work_OASACM3"
anymore.

@mickaelaccensi
Copy link
Copy Markdown
Collaborator Author

I've skipped toy_model in matrix.com
I've removed the unnecessary mkdir in matrix.base

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

So I ran just the ww3_tp2.14 tests again and compared against this branch and we get a lot of diffs:


********************* non-identical cases ****************************


ww3_tp2.14/./work_OASACM4 (210 files differ)
ww3_tp2.14/./work_OASICM (210 files differ)
ww3_tp2.14/./work_OASACM6 (210 files differ)
ww3_tp2.14/./work_OASOCM (210 files differ)
ww3_tp2.14/./work_OASACM2 (210 files differ)
ww3_tp2.14/./work_OASACM5 (210 files differ)
ww3_tp2.14/./work_OASACM (210 files differ)

a lot of work_oasis3-mct/build/* files.

@mickaelaccensi
Copy link
Copy Markdown
Collaborator Author

So I ran just the ww3_tp2.14 tests again and compared against this branch and we get a lot of diffs:

********************* non-identical cases ****************************

ww3_tp2.14/./work_OASACM4 (210 files differ) ww3_tp2.14/./work_OASICM (210 files differ) ww3_tp2.14/./work_OASACM6 (210 files differ) ww3_tp2.14/./work_OASOCM (210 files differ) ww3_tp2.14/./work_OASACM2 (210 files differ) ww3_tp2.14/./work_OASACM5 (210 files differ) ww3_tp2.14/./work_OASACM (210 files differ)

a lot of work_oasis3-mct/build/* files.

could you provide some logs to check what are the differences ?

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

@mickaelaccensi this is running this branch twice for the ww3_tp2.14 tests and then comparing the output:
matrixCompFull.txt

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

@mickaelaccensi I just tested that we have the same/similar issue with the cmake branch, if this works we can extend it assuming we just want to completely skip the "oasis3-mct" and "toy" directories. If you want to compare some of those files, we can also make that work

@mickaelaccensi
Copy link
Copy Markdown
Collaborator Author

it's fine to me. differences are only on compilation files and results are the same so we can merge

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

@mickaelaccensi we cannot merge because we will not be able to easily keep track of what files are different and if something breaks in the future. So we need to fix the comp script, which I think I might be able to have a fix for later today.

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

@mickaelaccensi I made a PR to this branch here: umr-lops#9 with what I think the matrix.comp updates need to be.

@ukmo-ccbunney
Copy link
Copy Markdown
Collaborator

Adding @ukmo-juan-castillo as a reviewer as he has more experience of running OASIS than me.
Juan - are you willing to do a review of Mickael's changes?

@ukmo-juan-castillo
Copy link
Copy Markdown
Collaborator

Yes, I can have a look at this tomorrow.

@ukmo-juan-castillo
Copy link
Copy Markdown
Collaborator

Changes look fine to me, just a couple of comments:

  1. It is not efficient compiling the oasis and toy model each time for every test, but this is something that will be addressed some day in issue Optimisations for coupled modelling performance (OASIS) #207
  2. The *.bot file is relatively large and it is repeated in all ww3_tp2.14 directories, it may be sensible to place it somewhere and copy it to each directory before running the test.
  3. The same as in 2. can be said about the script prep_env.sh (it looks like it is missing in the oasacm3 directory, I do not know if this is intentional). In this case it makes even more sense to have only one copy, as it is one script that may change and that would avoid having to change it in multiple locations, which can lead to errors.

@mickaelaccensi
Copy link
Copy Markdown
Collaborator Author

Changes look fine to me, just a couple of comments:

1. It is not efficient compiling the oasis and toy model each time for every test, but this is something that will be addressed some day in issue [Optimisations for coupled modelling performance (OASIS) #207](https://github.com/NOAA-EMC/WW3/issues/207)

I know but it will permit to run some of those tests in parallel since they are now independent

2. The *.bot file is relatively large and it is repeated in all ww3_tp2.14 directories, it may be sensible to place it somewhere and copy it to each directory before running the test.

good idea, I've moved it into input directory and updated the path in all ww3_grid.inp and ww3_grid.nml in others input_oas* directories

3. The same as in 2. can be said about the script prep_env.sh (it looks like it is missing in the oasacm3 directory, I do not know if this is intentional). In this case it makes even more sense to have only one copy, as it is one script that may change and that would avoid having to change it in multiple locations, which can lead to errors.

This is intentional for oasacm3 since it does run ww3 with oasis coupling, this work directory is only needed to prepare input for oasis4 directory.

Copy link
Copy Markdown
Collaborator

@ukmo-juan-castillo ukmo-juan-castillo left a comment

Choose a reason for hiding this comment

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

Everything seems in order, thanks for the answers to the points raised.

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

@ukmo-juan-castillo thank you for your review!

@mickaelaccensi can you merge in develop and I'll rerun the regtests since there were changes after I ran them last night. I expect no issues though.

@mickaelaccensi
Copy link
Copy Markdown
Collaborator Author

done

@JessicaMeixner-NOAA
Copy link
Copy Markdown
Collaborator

matrix.comp/run_test was updated so that we have more files in the other so many were "0 files differ"

The remaining differences of note are:

**********************************************************************
********************* non-identical cases ****************************
**********************************************************************
mww3_test_03/./work_PR2_UQ_MPI_d2                     (6 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (9 files differ)
mww3_test_03/./work_PR3_UNO_MPI_d2                     (6 files differ)
mww3_test_03/./work_PR2_UNO_MPI_d2                     (6 files differ)
mww3_test_03/./work_PR1_MPI_d2                     (8 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (9 files differ)
mww3_test_03/./work_PR3_UQ_MPI_d2                     (9 files differ)
ww3_tp2.10/./work_MPI_OMPH                     (7 files differ)
ww3_tp2.14/./work_OASACM4                     (1 files differ)
ww3_tp2.14/./work_OASACM3                     (1 files differ)
ww3_tp2.14/./work_OASACM6                     (5 files differ)
ww3_tp2.14/./work_OASOCM                     (1 files differ)
ww3_tp2.14/./work_OASACM2                     (1 files differ)
ww3_tp2.14/./work_OASACM5                     (1 files differ)
ww3_tp2.14/./work_OASACM                     (1 files differ)
ww3_tp2.16/./work_MPI_OMPH                     (6 files differ)
ww3_ufs1.3/./work_a                     (1 files differ)

Note it's expected that the tp2.14 files differ (log file mostly). I ran the ww3_tp2.14 tests a second time from this branch and the results are identical.

Another note, I got some sort of error so I actually had to remove the "-e" from the matrix.comp to get this script to finish, but we can address that later if it continues.

The full matrix files:

matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt

@JessicaMeixner-NOAA JessicaMeixner-NOAA merged commit 0496d36 into NOAA-EMC:develop Feb 17, 2022
aliabdolali pushed a commit that referenced this pull request Mar 16, 2022
and  fix it to use with namelist files
minor update in the coupling itself to use the same conditions for each exchanged variable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Regression Test A regression test is needed for a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

regtest ww3_tp2.14 not working properly with namelist

4 participants