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

Port 50R1 science contribution #59

Open
wants to merge 120 commits into
base: develop
Choose a base branch
from

Conversation

awnawab
Copy link
Contributor

@awnawab awnawab commented Jan 20, 2025

This PR brings over the 50R1 science contributions pertaining to the wave model.

The following testing has been completed:

  • Bit-identical norms with all output fields enabled if outblock is reverted to as it is in @cxzk's v8 branch
  • Bit-identical output to Zak's v8 branch in the IFS e-suite (@cxzk could you please check this again because since you last tested this branch there have been some ostensibly bit-identical contributions)

@jrbidlot and @jkousal32 could you please review this PR too? Please pay particular focus to the scripts changes, as unlike the source these are not patches directly generated from IFS commits.

The only thing missing in this PR are updates to the validation norms for the finer grids. I will add these as the final step before merging once the PR has been approved.

…te_wam_bathymetry_swldissip: 1) boost obstruction for high res to compensate for lack on resolution in subgrid data, 2) activate new swell dissip term to 50%
…XCEPT sbottom [sdice scales all previous source terms by ice cover beta]
…oved from SDICE to IMPLSCH and given switch LCISCAL
…ice: scattering, bottom friction & viscous friction
…ice: scattering, bottom friction & viscous friction
… which was probably much thicker than the values CITHICK is giving
@awnawab awnawab requested a review from wdeconinck January 20, 2025 20:24
@FussyDuck
Copy link

FussyDuck commented Jan 20, 2025

CLA assistant check
All committers have signed the CLA.

@awnawab awnawab requested a review from cxzk January 20, 2025 20:24
@jrbidlot
Copy link
Contributor

Hi Ahmad,
as far as I can tell it looks OK.
Note that there will be another minor contribution from the final 49R2, currently in the v8b branch but not yet in 50R1 (as far as I can tell)

@awnawab
Copy link
Contributor Author

awnawab commented Jan 23, 2025

Thanks for the feedback Jean! develop-1.4 will now be used to collect changes to 49R2. So changes just for 49R2 can go just there, if we decide to include it in 50R1 then we would simply cherry-pick those commits onto develop.

@awnawab
Copy link
Contributor Author

awnawab commented Jan 29, 2025

@cxzk were you able to run this branch through the CI again to confirm that it is bitid? @jkousal32 could you also please take a look at this PR?

@cxzk
Copy link
Contributor

cxzk commented Jan 29, 2025

@cxzk were you able to run this branch through the CI again to confirm that it is bitid? @jkousal32 could you also please take a look at this PR?

Trying to do that now, but I need to re-establish a clean reference first.

Copy link

@jkousal32 jkousal32 left a comment

Choose a reason for hiding this comment

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

All looks great to me, thanks for that!!

Copy link
Contributor

@cxzk cxzk left a comment

Choose a reason for hiding this comment

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

Yes, this appears to be bit-identical to my temporary 50r1 v8 + IFS-3792 branch, when run in standard IFS experiment configurations, so good to merge from my point of view and I can then switch to using this for the later stages of preparing CY50R1.

Thanks!

@awnawab
Copy link
Contributor Author

awnawab commented Jan 31, 2025

Thanks all for the reviews and feedback 🙏 @wdeconinck I have now updated the validation norms for the finer grids, so please merge this once the CI is green again. Could you also please tag this as 1.5.0?

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.

6 participants