fsd2 #394
fsd2 #394
Conversation
|
Travis CI is failing on the ftp data retrieval. I relaunched it and it failed again. |
There was a problem hiding this comment.
Does this change mean that wave_spectrum_profile is not used at all and could be removed from the code?
I agree we don't want the user to set wave_spec_type='none' and it be unexpectedly nonzero. However, having nonzero values does allow meaningful testing without a data file, and if I remember correctly, we only have the data in netCDF, not binary format. So perhaps we should keep the profile and have these options for wave_spec_type:
none
profile
constant
random
Does that make sense?
|
Yes OK, I agree - adding the 'profile' option is a good idea.
…On Sat, Jan 11, 2020, 03:11 Elizabeth Hunke ***@***.***> wrote:
***@***.**** commented on this pull request.
Does this change mean that wave_spectrum_profile is not used at all and
could be removed from the code?
I agree we don't want the user to set wave_spec_type='none' and it be
unexpectedly nonzero. However, having nonzero values does allow meaningful
testing without a data file, and if I remember correctly, we only have the
data in netCDF, not binary format. So perhaps we should keep the profile
and have these options for wave_spec_type:
none
profile
constant
random
Does that make sense?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#394?email_source=notifications&email_token=AGIYTCLWY6SBP2HT242F3H3Q5GSMRA5CNFSM4KFOUJCKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCRNSJZA#pullrequestreview-341517540>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGIYTCJLDJHNTEH3GZEDERDQ5GSMRANCNFSM4KFOUJCA>
.
|
There was a problem hiding this comment.
The code changes look fine to me, except there is a change to Icepack showing up that's probably not intended. Will these changes result in non-BFB test results for the ww3 test case? It's fine if they do, since this is fixing a bug. @lettie-roach do you have your computational environment set up to be able to run the test suites, or at least the fsd test cases, and compare with the code without your mods? We'll need that done for this PR.
|
There shouldn't be changes to Icepack. The ones in the diff aren't mine -
maybe I'm out of sync with the master. I don't know how to run the test
suites. I don't think it will be BFB since the options are different
…On Mon, 13 Jan 2020 at 10:13, Elizabeth Hunke ***@***.***> wrote:
***@***.**** commented on this pull request.
The code changes look fine to me, except there is a change to Icepack
showing up that's probably not intended. Will these changes result in
non-BFB test results for the ww3 test case? It's fine if they do, since
this is fixing a bug. @lettie-roach <https://github.com/lettie-roach> do
you have your computational environment set up to be able to run the test
suites, or at least the fsd test cases, and compare with the code without
your mods? We'll need that done for this PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#394?email_source=notifications&email_token=AGIYTCPKIJAWT5TDA6M3ZZLQ5SVNTA5CNFSM4KFOUJCKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCRRRTMY#pullrequestreview-342038963>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGIYTCLQXTQWQDP6VWKJRZ3Q5SVNTANCNFSM4KFOUJCA>
.
|
|
Okay, then you have something to learn during the tutorial! I'll try to get a set of tests running here this week. I'll let @apcraig figure out the Icepack thing and make a recommendation for what to do. Thanks! |
|
Ran the base_suite and compared with the current master. The fsd12 (no ww3) tests fails because of the namelist change, as expected. The ‘robin’ decomposition tests are failing for an unknown reason, but the fsd changes should not be contributing; both appear to have dropped nodes and timed out. The JRA runs fails because the data is still unavailable. I think these results are acceptable and the PR is ready to merge. |
|
@apcraig please double-check that the Icepack change is desirable in this commit. Thx |
PR checklist
Correction to 'wave_spec_type' logic
@lettie-roach
@eclare108213 @apcraig @duvivier
https://github.com/CICE-Consortium/Test-Results/wiki/c52a091233.badger.intel.20-01-24.033842
225 of 236 tests PASSED
10 of 236 tests FAILED
0 of 236 tests PENDING
See explanation below (@eclare108213).
Correction to 'wave_spec_type' - remove 'file' option (redundant). Updated documentation.
I also changed the code so that if wave_spec_type = 'none', then the wave spectrum is set to zero. Previously, we used a dummy fixed wave spectrum in this case, which was useful for code testing, but I think should be zeroed out for the general user.
The FSD physics is unchanged, but the logic for the wave forcing has been corrected, which changes the answers for the fsd12 test option. @eclare108213 changed the fsd12 test because it was aborting due to the new wave_spec_type logic.