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

62 z remapping uv grids #191

Merged
merged 18 commits into from
Jul 28, 2015

Conversation

nichannah
Copy link
Collaborator

It does the remapping for all variables on u, v grids. Altogether there are > 100 new diagnostics being remapped to Z coordinates.

This branch includes 62-z-ALE-remapping so it could be merged instead of 62-z-ALE-remapping. There a couple things that I'm not sure about in this branch:

  1. it doesn't handle variables on the B grid, I'm not sure whether I'm doing this properly, the remapping fails with large errors. So I've just disabled those axes for the time being
  2. I've had to mess with the remapping error checking a bit. I've introduced maximum (hard coded) relative and absolute errors, it's not very nice.

@nichannah nichannah force-pushed the 62-z-remapping-uv-grids branch from 734d9f1 to 56b2a20 Compare July 13, 2015 22:13
@nichannah nichannah force-pushed the 62-z-remapping-uv-grids branch from 43282ce to 8bc1092 Compare July 15, 2015 14:46
@nichannah nichannah force-pushed the 62-z-remapping-uv-grids branch from 218f75e to 7a990db Compare July 16, 2015 18:01
@nichannah nichannah force-pushed the 62-z-remapping-uv-grids branch from 7a990db to 49cd415 Compare July 16, 2015 21:15
@nichannah nichannah force-pushed the 62-z-remapping-uv-grids branch from 9f8134d to bc8a185 Compare July 16, 2015 22:44
@nichannah
Copy link
Collaborator Author

This PR is now ready to review and merge. There should be no results change.

@@ -72,7 +72,7 @@ module MOM_continuity

contains

subroutine continuity(u, v, hin, h, uh, vh, dt, G, CS, uhbt, vhbt, OBC, &
subroutine continuity(u, v, hin, h, uh, vh, dt, G, CS, diag_cs, uhbt, vhbt, OBC, &
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you move the call to diag_update_target_grids() to just after the call to continuity() so that we don't need the diag_CS type passed into continuity().

This should give just a few less calls to diag_update_target_grids() but more importantly keeps continuity look more like a computational kernel without diagnostic.

continuity(). It is not necessary to pass the diag mediator structure
into calls to continuity(). mom-ocean#62
@nichannah
Copy link
Collaborator Author

I have moved the calls to diag_update_target_grids() out of continuity(). This has the benefits of keeping the continuity() interface clean, and reducing unnecessary calls to diag_update_target_grids().

@adcroft can you please review etc. when you get time.

call MOM_error( FATAL, 'MOM_remapping, remapping_core: '//&
'Total stuff on h0 and hF differ by more than roundoff' )
! Maximum relative error
if (abs(totalHU2-totalHU0) / totalHU2 > 1e-09) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

I gather the remapping was not accurate enough? I was quite proud of the estimate of round-off and haven't found the criteria to fail so far. Perhaps a left over from debugging?

@adcroft adcroft merged commit d0e99df into mom-ocean:dev/master Jul 28, 2015
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