Add buoyancy flux to applyBoundaryFluxes() #543
Labels
Comments
Merged
|
I have been working on an alternative fix to 128 that is in line with what
you describe here. I should be able to push it up tomorrow. I've got it
working in a way that doesn't change answers for now, but will include
comments in the code for some details we still need to think about.
…On Sun, Jul 2, 2017 at 4:17 PM, Alistair Adcroft ***@***.***> wrote:
In issue NOAA-GFDL/MOM6-examples#128
<NOAA-GFDL/MOM6-examples#128> it was noted by
@StephenGriffies <https://github.com/stephengriffies> that multiple calls
to extractFluxes1d() is wasteful (and in that issue was causing
complications with diagnostics). A second call to extractFluxes1d() is
being made in order to calculate a buoyancy flux. Note that this second
call has different arguments/options hard-coded. We would be better served
calling extractFluxes1d() just once from applyBoundaryFluxes() and
calculating a buoyancy flux there.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<NOAA-GFDL#543>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJQH2HCalupMnP3Hmvk8XsBrHRItHHm0ks5sJ_rBgaJpZM4OLrh8>
.
|
|
Looking forward to your cleaner fix. #544 does not remove need to clean
this up. You might need to resolve some conflicts though.
…--
Dr Alistair Adcroft (Alistair.Adcroft@noaa.gov)
Princeton University Tel: (609) 987-5073
NOAA/GFDL, 201 Forrestal Road, Princeton, NJ 08540
On Sun, Jul 2, 2017 at 8:31 PM, Brandon Reichl <notifications@github.com>
wrote:
I have been working on an alternative fix to 128 that is in line with what
you describe here. I should be able to push it up tomorrow. I've got it
working in a way that doesn't change answers for now, but will include
comments in the code for some details we still need to think about.
On Sun, Jul 2, 2017 at 4:17 PM, Alistair Adcroft ***@***.***
>
wrote:
> In issue NOAA-GFDL/MOM6-examples#128
> <NOAA-GFDL/MOM6-examples#128> it was noted by
> @StephenGriffies <https://github.com/stephengriffies> that multiple
calls
> to extractFluxes1d() is wasteful (and in that issue was causing
> complications with diagnostics). A second call to extractFluxes1d() is
> being made in order to calculate a buoyancy flux. Note that this second
> call has different arguments/options hard-coded. We would be better
served
> calling extractFluxes1d() just once from applyBoundaryFluxes() and
> calculating a buoyancy flux there.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <NOAA-GFDL#543>, or mute the thread
> <https://github.com/notifications/unsubscribe-auth/
AJQH2HCalupMnP3Hmvk8XsBrHRItHHm0ks5sJ_rBgaJpZM4OLrh8>
> .
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<NOAA-GFDL#543 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AFlo84cQip8FvFalMB10mwJ_R5CTl9Swks5sKDZIgaJpZM4OLrh8>
.
|
|
Should I wait for you to merge 544 so I can update my changes for the
conflicts?
BTW, my fix isn't the cleanest, but it does eliminate the second call to
extractfluxes1d while taking some necessary steps to avoid changing
answers. I'll push the code up once you let me know your preference, but
FYI what I have done is:
1) Add additional (optional) outputs to extractfluxes1d of netheat,
netsalt, netMassInOut, and pen_sw_bnd that are prescribed to use (a) dt=1
(required to avoid answer change due to round-off because the call in
applyboundaryfluxesinout uses the actual value of dt) and (b) the
hard-coded choices for using river/calving fluxes. Then I add a loop at
the end of applyboundaryfluxesinout to compute the (now 2d) surface
buoyancy flux array from those output rates. The loop in
applyboundaryfluxesinout is only called if the optional output (the 2d
surface buoyancy flux) is requested.
Thus, with the optional arguments present extractfluxes1d is only called
once and answers are not changed. However, if we accept answer changes due
to round-off, we do not need the additional dt=1 outputs of netsalt,
netmassinout, and pen_sw_bnd; but we still need the additional netheat
output to account for the difference in river/calving heat fluxes flags in
the OM4_05 test case (the quarter degree has these set to false, which
perhaps should be changed to true at somepoint).
On Mon, Jul 3, 2017 at 12:59 PM, Alistair Adcroft <notifications@github.com>
wrote:
… Looking forward to your cleaner fix. #544 does not remove need to clean
this up. You might need to resolve some conflicts though.
--
Dr Alistair Adcroft ***@***.***)
Princeton University Tel: (609) 987-5073
NOAA/GFDL, 201 Forrestal Road, Princeton, NJ 08540
On Sun, Jul 2, 2017 at 8:31 PM, Brandon Reichl ***@***.***>
wrote:
> I have been working on an alternative fix to 128 that is in line with
what
> you describe here. I should be able to push it up tomorrow. I've got it
> working in a way that doesn't change answers for now, but will include
> comments in the code for some details we still need to think about.
>
> On Sun, Jul 2, 2017 at 4:17 PM, Alistair Adcroft <
***@***.***
> >
> wrote:
>
> > In issue NOAA-GFDL/MOM6-examples#128
> > <NOAA-GFDL/MOM6-examples#128> it was noted
by
> > @StephenGriffies <https://github.com/stephengriffies> that multiple
> calls
> > to extractFluxes1d() is wasteful (and in that issue was causing
> > complications with diagnostics). A second call to extractFluxes1d() is
> > being made in order to calculate a buoyancy flux. Note that this second
> > call has different arguments/options hard-coded. We would be better
> served
> > calling extractFluxes1d() just once from applyBoundaryFluxes() and
> > calculating a buoyancy flux there.
> >
> > —
> > You are receiving this because you are subscribed to this thread.
> > Reply to this email directly, view it on GitHub
> > <NOAA-GFDL#543>, or mute the thread
> > <https://github.com/notifications/unsubscribe-auth/
> AJQH2HCalupMnP3Hmvk8XsBrHRItHHm0ks5sJ_rBgaJpZM4OLrh8>
> > .
> >
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <NOAA-GFDL#543 (comment)>,
or mute
> the thread
> <https://github.com/notifications/unsubscribe-
auth/AFlo84cQip8FvFalMB10mwJ_R5CTl9Swks5sKDZIgaJpZM4OLrh8>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<NOAA-GFDL#543 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AJQH2MsxWeGz_qdQOrLwIxTMKhnXM-nqks5sKR35gaJpZM4OLrh8>
.
|
|
It would be simplest to wait for the merges. We're all waiting for Bob to
click the "Merge Pull Request" button on this and other requests.
Is it truly round-off difference for OM4_05? You'll have to ask John D.
whether he'll accept that.
I agree about turning on river heat fluxes. I need to put a run in with
this.
…--
Dr Alistair Adcroft (Alistair.Adcroft@noaa.gov)
Princeton University Tel: (609) 987-5073
NOAA/GFDL, 201 Forrestal Road, Princeton, NJ 08540
On Mon, Jul 3, 2017 at 1:46 PM, Brandon Reichl <notifications@github.com>
wrote:
Should I wait for you to merge 544 so I can update my changes for the
conflicts?
BTW, my fix isn't the cleanest, but it does eliminate the second call to
extractfluxes1d while taking some necessary steps to avoid changing
answers. I'll push the code up once you let me know your preference, but
FYI what I have done is:
1) Add additional (optional) outputs to extractfluxes1d of netheat,
netsalt, netMassInOut, and pen_sw_bnd that are prescribed to use (a) dt=1
(required to avoid answer change due to round-off because the call in
applyboundaryfluxesinout uses the actual value of dt) and (b) the
hard-coded choices for using river/calving fluxes. Then I add a loop at
the end of applyboundaryfluxesinout to compute the (now 2d) surface
buoyancy flux array from those output rates. The loop in
applyboundaryfluxesinout is only called if the optional output (the 2d
surface buoyancy flux) is requested.
Thus, with the optional arguments present extractfluxes1d is only called
once and answers are not changed. However, if we accept answer changes due
to round-off, we do not need the additional dt=1 outputs of netsalt,
netmassinout, and pen_sw_bnd; but we still need the additional netheat
output to account for the difference in river/calving heat fluxes flags in
the OM4_05 test case (the quarter degree has these set to false, which
perhaps should be changed to true at somepoint).
On Mon, Jul 3, 2017 at 12:59 PM, Alistair Adcroft <
***@***.***>
wrote:
> Looking forward to your cleaner fix. #544 does not remove need to clean
> this up. You might need to resolve some conflicts though.
>
> --
> Dr Alistair Adcroft ***@***.***)
> Princeton University Tel: (609) 987-5073
> NOAA/GFDL, 201 Forrestal Road, Princeton, NJ 08540
>
> On Sun, Jul 2, 2017 at 8:31 PM, Brandon Reichl ***@***.***
>
> wrote:
>
> > I have been working on an alternative fix to 128 that is in line with
> what
> > you describe here. I should be able to push it up tomorrow. I've got it
> > working in a way that doesn't change answers for now, but will include
> > comments in the code for some details we still need to think about.
> >
> > On Sun, Jul 2, 2017 at 4:17 PM, Alistair Adcroft <
> ***@***.***
> > >
> > wrote:
> >
> > > In issue NOAA-GFDL/MOM6-examples#128
> > > <NOAA-GFDL/MOM6-examples#128> it was noted
> by
> > > @StephenGriffies <https://github.com/stephengriffies> that multiple
> > calls
> > > to extractFluxes1d() is wasteful (and in that issue was causing
> > > complications with diagnostics). A second call to extractFluxes1d()
is
> > > being made in order to calculate a buoyancy flux. Note that this
second
> > > call has different arguments/options hard-coded. We would be better
> > served
> > > calling extractFluxes1d() just once from applyBoundaryFluxes() and
> > > calculating a buoyancy flux there.
> > >
> > > —
> > > You are receiving this because you are subscribed to this thread.
> > > Reply to this email directly, view it on GitHub
> > > <NOAA-GFDL#543>, or mute the thread
> > > <https://github.com/notifications/unsubscribe-auth/
> > AJQH2HCalupMnP3Hmvk8XsBrHRItHHm0ks5sJ_rBgaJpZM4OLrh8>
> > > .
> > >
> >
> > —
> > You are receiving this because you authored the thread.
> > Reply to this email directly, view it on GitHub
> > <NOAA-GFDL#543 (comment)>,
> or mute
> > the thread
> > <https://github.com/notifications/unsubscribe-
> auth/AFlo84cQip8FvFalMB10mwJ_R5CTl9Swks5sKDZIgaJpZM4OLrh8>
> > .
> >
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <NOAA-GFDL#543 (comment)>,
or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AJQH2MsxWeGz_
qdQOrLwIxTMKhnXM-nqks5sKR35gaJpZM4OLrh8>
> .
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<NOAA-GFDL#543 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AFlo86SZ2R3We-dZD5dbNY9NdXf3LzZeks5sKSjogaJpZM4OLrh8>
.
|
|
No problem then, I'll hold off.
*Is it truly round-off difference for OM4_05? You'll have to ask John D.
whether he'll accept that.*
Yes, several of the additional outputs are only required by round-off. I
could alternatively take the netsalt, netmassinout, and pen_SW_bnd ouput
from extractfluxes1d and divide by the DT value it is called with. But,
unfortunately N*3600./3600. does not quite equal N. I talked to Bob and he
was willing to accept this, but since I have to add an extra netheat output
regardless I just included all 4 outputs for now (to satisfy the previous
answers).
On Mon, Jul 3, 2017 at 1:56 PM, Alistair Adcroft <notifications@github.com>
wrote:
… It would be simplest to wait for the merges. We're all waiting for Bob to
click the "Merge Pull Request" button on this and other requests.
Is it truly round-off difference for OM4_05? You'll have to ask John D.
whether he'll accept that.
I agree about turning on river heat fluxes. I need to put a run in with
this.
--
Dr Alistair Adcroft ***@***.***)
Princeton University Tel: (609) 987-5073
NOAA/GFDL, 201 Forrestal Road, Princeton, NJ 08540
On Mon, Jul 3, 2017 at 1:46 PM, Brandon Reichl ***@***.***>
wrote:
> Should I wait for you to merge 544 so I can update my changes for the
> conflicts?
>
> BTW, my fix isn't the cleanest, but it does eliminate the second call to
> extractfluxes1d while taking some necessary steps to avoid changing
> answers. I'll push the code up once you let me know your preference, but
> FYI what I have done is:
>
> 1) Add additional (optional) outputs to extractfluxes1d of netheat,
> netsalt, netMassInOut, and pen_sw_bnd that are prescribed to use (a) dt=1
> (required to avoid answer change due to round-off because the call in
> applyboundaryfluxesinout uses the actual value of dt) and (b) the
> hard-coded choices for using river/calving fluxes. Then I add a loop at
> the end of applyboundaryfluxesinout to compute the (now 2d) surface
> buoyancy flux array from those output rates. The loop in
> applyboundaryfluxesinout is only called if the optional output (the 2d
> surface buoyancy flux) is requested.
>
> Thus, with the optional arguments present extractfluxes1d is only called
> once and answers are not changed. However, if we accept answer changes
due
> to round-off, we do not need the additional dt=1 outputs of netsalt,
> netmassinout, and pen_sw_bnd; but we still need the additional netheat
> output to account for the difference in river/calving heat fluxes flags
in
> the OM4_05 test case (the quarter degree has these set to false, which
> perhaps should be changed to true at somepoint).
>
> On Mon, Jul 3, 2017 at 12:59 PM, Alistair Adcroft <
> ***@***.***>
> wrote:
>
> > Looking forward to your cleaner fix. #544 does not remove need to clean
> > this up. You might need to resolve some conflicts though.
> >
> > --
> > Dr Alistair Adcroft ***@***.***)
> > Princeton University Tel: (609) 987-5073
> > NOAA/GFDL, 201 Forrestal Road, Princeton, NJ 08540
> >
> > On Sun, Jul 2, 2017 at 8:31 PM, Brandon Reichl <
***@***.***
> >
> > wrote:
> >
> > > I have been working on an alternative fix to 128 that is in line with
> > what
> > > you describe here. I should be able to push it up tomorrow. I've got
it
> > > working in a way that doesn't change answers for now, but will
include
> > > comments in the code for some details we still need to think about.
> > >
> > > On Sun, Jul 2, 2017 at 4:17 PM, Alistair Adcroft <
> > ***@***.***
> > > >
> > > wrote:
> > >
> > > > In issue NOAA-GFDL/MOM6-examples#128
> > > > <NOAA-GFDL/MOM6-examples#128> it was
noted
> > by
> > > > @StephenGriffies <https://github.com/stephengriffies> that
multiple
> > > calls
> > > > to extractFluxes1d() is wasteful (and in that issue was causing
> > > > complications with diagnostics). A second call to extractFluxes1d()
> is
> > > > being made in order to calculate a buoyancy flux. Note that this
> second
> > > > call has different arguments/options hard-coded. We would be better
> > > served
> > > > calling extractFluxes1d() just once from applyBoundaryFluxes() and
> > > > calculating a buoyancy flux there.
> > > >
> > > > —
> > > > You are receiving this because you are subscribed to this thread.
> > > > Reply to this email directly, view it on GitHub
> > > > <NOAA-GFDL#543>, or mute the thread
> > > > <https://github.com/notifications/unsubscribe-auth/
> > > AJQH2HCalupMnP3Hmvk8XsBrHRItHHm0ks5sJ_rBgaJpZM4OLrh8>
> > > > .
> > > >
> > >
> > > —
> > > You are receiving this because you authored the thread.
> > > Reply to this email directly, view it on GitHub
> > > <NOAA-GFDL#543 (comment)
>,
> > or mute
> > > the thread
> > > <https://github.com/notifications/unsubscribe-
> > auth/AFlo84cQip8FvFalMB10mwJ_R5CTl9Swks5sKDZIgaJpZM4OLrh8>
> > > .
> > >
> >
> > —
> > You are receiving this because you commented.
> > Reply to this email directly, view it on GitHub
> > <NOAA-GFDL#543 (comment)>,
> or mute
> > the thread
> > <https://github.com/notifications/unsubscribe-auth/AJQH2MsxWeGz_
> qdQOrLwIxTMKhnXM-nqks5sKR35gaJpZM4OLrh8>
> > .
> >
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <NOAA-GFDL#543 (comment)>,
or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AFlo86SZ2R3We-
dZD5dbNY9NdXf3LzZeks5sKSjogaJpZM4OLrh8>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<NOAA-GFDL#543 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AJQH2JySmLQw3_5Bkk3srYzhUZ7M_zdIks5sKStqgaJpZM4OLrh8>
.
|
|
When I return after a brief holiday, I will try to check what you already ran, Brandon, to see if the hfds diagnostic is correct based on your changes. |
|
@breichl any reason to keep this issue open? Can you please check? |
|
I left some notes about this in the code and a commit, which concluded with "This commit does not change answers, but similar to #544, this is not a permanent fix as the code could be simplified to remove '_rate' terms (will introduce round-off answer changes) and a more permanent solution for useRiverHeatContent and useCalvingHeatContent. These points are described within code comments." We have a protocol established now of adding flags with the new code that removes the round-off error associated with the duplicate call (to get the '_rate' terms), and then changing answers and deleting old code. So I think this should probably be kept open for now, and I'll add the flags to clean that part up. I'm unsure what the issues were with useRiverHeatContent and useCalvingHeatContent, so will revisit this when I'm looking at the code again. |
|
Thanks @breichl |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In issue NOAA-GFDL/MOM6-examples#128 it was noted by @StephenGriffies that multiple calls to extractFluxes1d() is wasteful (and in that issue was causing complications with diagnostics). A second call to extractFluxes1d() is being made in order to calculate a buoyancy flux. Note that this second call has different arguments/options hard-coded. We would be better served calling extractFluxes1d() just once from applyBoundaryFluxes() and calculating a buoyancy flux there.
The text was updated successfully, but these errors were encountered: