Skip to content

Update areafac_c, areafac_ce in halo in dynamics#1

Merged
JFLemieux73 merged 1 commit into
JFLemieux73:Areafactfrom
apcraig:far3
Sep 14, 2023
Merged

Update areafac_c, areafac_ce in halo in dynamics#1
JFLemieux73 merged 1 commit into
JFLemieux73:Areafactfrom
apcraig:far3

Conversation

@apcraig
Copy link
Copy Markdown

@apcraig apcraig commented Sep 14, 2023

I believe this fixes the issues in the updated remap advection. The problem was that CICE was not bit-for-bit on different block sizes with the latest changes even though it was bit-for-bit for a fixed block size with different pes or decompositions. This indicated a problem on the halo. The problem existed with B, C, or CD grids in this branch.

The fix is to change the areafac_c and areafac_ce fields so they are defined everywhere, including in the complete halo. The current version only sets areafac_c and areafac_ce on part of the halo (for reasons I don't understand). earea and narea are defined everywhere on the halo, so we can set areafac_c and areafac_ce in the complete halo trivially.

The changes in ice_grid.F90 are not required for this fix, but are probably needed for symmetry testing (which we don't do automatically yet).

Somewhere in the remap advection special cases, areafac_c or (more likely) areafac_ce must have been used on a point where it wasn't set properly on the halo. And that's why different block sizes produced different answers. With these fixes, that problem goes away. That doesn't guarantee that all the special cases are implemented correctly in terms of offsets and answers, but this does fix the block size reproducibility issue.

I'm running a complete test suite now and will have more results soon. Feel free to merge and/or test this in the areafact branch in the mean time. I think it's safe to do so. I will run a complete test suite, we probably want to also rerun the QCs and any other manual testing just be really sure the final thing we want to merge is OK.

@JFLemieux73
Copy link
Copy Markdown
Owner

Thank you very much Tony. I am not sure I understand everything but I will merge this and redo the tests.

@JFLemieux73 JFLemieux73 merged commit 1a4fd67 into JFLemieux73:Areafact Sep 14, 2023
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.

2 participants