[MITgcm-devel] naming for inAdMode switches
Jean-Michel Campin
jmc at ocean.mit.edu
Tue Nov 13 14:33:57 EST 2012
Hi Martin and others,
OK, I admit my explanations were not very clear.
If we go with solution (b) (which I tend to prefer):
- we can keep Real and Integer flags as $(PARMNAME)inFwdMode
and $(PARMNAME)inAdMode (since there is always a way to initialise
$(PARMNAME)inAdMode to an unlikely value; done in autodiff_readparms.F,
before reading the namelist AUTODIFF_PARM01).
- but for logical flags, we would keep also $(PARMNAME)inFwdMode but
will need to have a $(PARMNAME)switchInAd (or switchInAd$(PARMNAME))
which will reverse the value of $(PARMNAME) once we are in Ad-Mode.
- the same problem does not really exist for run-time params that turns
on/off a full pkg (e.g., useKPP) since they are read-in much earlier,
from data.pkg, and the code we have now works with no problem. So we can
decide to keep the logic and the names we currently have (e.g,
useKPPinFwdMode & useKPPinAdMode).
In term of changes, the new parameters will be:
SEAICEuseDYNAMICSswitchInAd (in namelist AUTODIFF_PARM01)
SEAICEuseFREEDRIFTswitchInAd (in namelist AUTODIFF_PARM01)
SEAICEuseFREEDRIFTinFwdMode (not in any namelist)
we will keep:
SEAICEuseDYNAMICSinFwdMode (not in any namelist)
and both SEAICEuseDYNAMICSinAdMode and turnFreeDriftInAdMode will be removed.
If we go with solution (a), we need to move the call to AUTODIFF_READPARMS
to the last position in the sequence of calls of packages_readparms.F
(and assume we will never have to move it earlier); but the parameter
names can stay as thery are now: $(PARMNAME)inFwdMode & $(PARMNAME)inAdMode
(even for logical $(PARMNAME parameters). turnFreeDriftInAdMode would need
to be replaced by SEAICEuseFREEDRIFTinFwdMode + SEAICEuseFREEDRIFTinAdMode
Cheers,
Jean-Michel
On Fri, Nov 02, 2012 at 08:28:10AM +0100, Martin Losch wrote:
> Hi Jean-Michel, Patrick,
>
> I think I would go with Jean-Michel's option (b) (provided that I understand his email correctly after reading it at least 4 times (o:),
> and I also think that the naming convention $(PARMNAME)inFwdMode and $(PARMNAME)inAdMode is useful and intuitive and we should maintain it. (or adopt it)
>
> Martin
>
> On Oct 27, 2012, at 11:48 PM, Jean-Michel Campin wrote:
>
> > Hi Patrick,
> >
> > There was a problem of SEAICEuseDYNAMICSinAdMode being set to True
> > even with SEAICEuseDYNAMICS=F in data.seaice,
> > and 1 verification experiment (lab_sea/input_ad.noseaicedyn)
> > was failing.
> >
> > But regarding your 1rst email:
> >
> > 1) I agree that the turnFreeDriftInAdMode code needs to be changed.
> > but I would prefer to maintain our naming convention (SEAICEuseDYNAMICSinAdMode
> > does not fit since it's not stored in SEAICE*.h header file but in
> > AUTODIFF_PARAMS.h ! inAdModeSEAICEuseDYNAMICS name would fit better)
> > 2) we should do something cleaner if, in packages_readparms.F,
> > the position of CALL AUTODIFF_READPARMS within the sequence of other
> > PKGs read_params calls matters.
> > I see 2 options:
> > a) move the call to AUTODIFF_READPARMS in the last position in
> > packages_readparms.F
> > but: (I) it is still not perfect, since there are params which are set or
> > reset later on, until set_params.F is called.
> > (II) might become a problem if , for some reason, 1 pkg/autodiff params
> > has to be known early on.
> > b) split AUTODIFF_READPARMS in 2 parts: 1rst part just to read data.autodiff
> > and 2nd part (i,e, 2nd S/R), which can be called from set_params.F, to set
> > the values of XXXinFwdMode and XXXinAdMode (or the equivalent of XXXinAdMode).
> >
> > 3) Although is not yet an issue (not too many of these switches),
> > could be a good thing to try to separate different packages (e.g.,
> > not to have to include all parameter header files in autodiff_readparms.F)
> > I don't think it would be too difficult to maintain pkg separation in
> > autodiff_inadmode_set/unset_ad.F (just need to have 1 small S/R per pkg
> > to set/reset XXX); but regarding autodiff_readparms.F, it might be
> > more tricky if the start in the wrong direction.
> >
> > 4) we need to be careful with logical parameters, because setting
> > the default of XXXinAdMode to True (or False) before reading data.autodiff
> > might not work if we want to allow to switch logical flag XXX in
> > adjoint mode while keeping it's current value in Fwd mode, but not to
> > change anything when using the default (empty namelist).
> > With solution (a), we would do:
> > XXXinAdMode = XXX
> > read XXXinAdMode from data.autodiff
> > and then just set:
> > XXXinFwdMode = XXX
> > this might make it more difficult to maintain pkg separation (point 3).
> > But with solution (b), we should do:
> > switchXXXinAdMode = FALSE
> > read switchXXXinAdMode from data.autodiff
> > and in 2nd S/R:
> > XXXinFwdMode = XXX
> > and in autodiff_inadmode_set_ad.F:
> > IF ( switchXXXinAdMode ) XXX = .NOT.XXXinFwdMode
> > and in autodiff_inadmode_unset_ad.F:
> > IF ( switchXXXinAdMode ) XXX = XXXinFwdMode
> >
> > And finally, solution (b) can be simplified (no need for 2nd S/R)
> > if we store directly XXXinFwdMode = XXX from ${PKG}_readparams.F
> > (just need to include AUTODIFF_PARAMS.h there).
> > And with solution (a), to maintain pkg separation, the storage + initialisation
> > of XXXinAdMode and XXXinFwdMode can also be moved to ${PKG}_readparams.F
> > (again, just need to include AUTODIFF_PARAMS.h there).
> >
> > I tend to prefer solution (b), but would not be a big deal if we decide to
> > go with (a).
> >
> > Cheers,
> > Jean-Michel
> >
> > On Sat, Oct 27, 2012 at 03:21:41PM -0400, Patrick Heimbach wrote:
> >>
> >> Hi Jean-Michel,
> >>
> >> I just saw you added following comment, and it fits the discussion thread I started yesterday:
> >>
> >> #ifdef ALLOW_SEAICE
> >> 104 C- Note: should re-visit how to turn on/off pkg internal-params in adjoint mode
> >> 105 C This will not work for PKGs which load their params AFTER this S/R is called
> >> 106 C (currently, it is OK for SEAICE but not for all PKGs !)
> >> 107 SEAICEuseDYNAMICSinFwdMode = SEAICEuseDYNAMICS
> >> 108 #endif
> >>
> >> I'm aware of this problem, but it doesn't change the fact that we will need to have this capability (e.g., change viscosity values, potentially change advection schemes - if that works, etc). I've been trying to point out this issue back when the first discussions started on changing autodiff_inadmode_set..., addind data.autodiff, etc.
> >> Perhaps we can try to include some checks to ensure that indeed those data.<pkg> values have been read previously.
> >>
> >>
> >>
> >> On Oct 26, 2012, at 8:50 PM, Patrick Heimbach <heimbach at mit.edu> wrote:
> >>
> >>>
> >>> Hi All,
> >>>
> >>> in order to allow more switches in autodiff_inadmode_set_ad
> >>> I would like to change the naming and assigning of current seaice parameter
> >>> turnFreeDriftInAdMode somewhat:
> >>> Naming should be SEAICEuseFREEDRIFTinAdMode
> >>> and should be assigned like the newly introduced parameter SEAICEuseDYNAMICSinAdMode
> >>> One reason is to be more easily searchable/grep-able in conjunction with its regular parameter.
> >>>
> >>> I realize that the current logic for turnFreeDriftInAdMode has the advantage of not requiring
> >>> including SEAICE_PARAMS.h in autodiff_readparams.F, but:
> >>> 1. it's a special case;
> >>> 2. it's not clear what "turn" means (switch?);
> >>> 3. sooner or later we'll need to add parameter headers anyways, e.g. when we want to temporarily change values of parameters during adjoint leg (which we already do in certain applications).
> >>> So we might as well start early to bring some consistency into parameter naming
> >>> (clean identification between regular parameter name and its inAdMode inFwdMode counterpart).
> >>>
> >>> Any objection?
> >>>
> >>> Cheers
> >>> p.
> >>>
> >>> Begin forwarded message:
> >>>
> >>>> From: Patrick Heimbach <heimbach at forge.csail.mit.edu>
> >>>> Subject: [MITgcm-cvs] MITgcm/pkg/autodiff CVS Commit
> >>>> Date: October 26, 2012 8:38:21 PM EDT
> >>>> To: mitgcm-cvs at mitgcm.org
> >>>> Reply-To: MITgcm-cvs at mitgcm.org
> >>>>
> >>>> Update of /u/gcmpack/MITgcm/pkg/autodiff
> >>>> In directory forge:/tmp/cvs-serv21710
> >>>>
> >>>> Modified Files:
> >>>> AUTODIFF_PARAMS.h autodiff_inadmode_set_ad.F
> >>>> autodiff_inadmode_unset_ad.F autodiff_readparms.F
> >>>> Log Message:
> >>>> Add inAdMode switch for SEAICEuseDYNAMICS
> >>>> (extend logic in ADAUTODIFF_INADMODE_SET)
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> MITgcm-cvs mailing list
> >>>> MITgcm-cvs at mitgcm.org
> >>>> http://mitgcm.org/mailman/listinfo/mitgcm-cvs
> >>>
> >>>
> >>> ---
> >>> Patrick Heimbach | heimbach at mit.edu | http://www.mit.edu/~heimbach
> >>> MIT | EAPS 54-1420 | 77 Massachusetts Ave | Cambridge MA 02139 USA
> >>> FON +1-617-253-5259 | FAX +1-617-253-4464 | SKYPE patrick.heimbach
> >>>
> >>> _______________________________________________
> >>> MITgcm-devel mailing list
> >>> MITgcm-devel at mitgcm.org
> >>> http://mitgcm.org/mailman/listinfo/mitgcm-devel
> >>
> >>
> >> ---
> >> Patrick Heimbach | heimbach at mit.edu | http://www.mit.edu/~heimbach
> >> MIT | EAPS 54-1420 | 77 Massachusetts Ave | Cambridge MA 02139 USA
> >> FON +1-617-253-5259 | FAX +1-617-253-4464 | SKYPE patrick.heimbach
> >>
> >
> >
> >
> >> _______________________________________________
> >> MITgcm-devel mailing list
> >> MITgcm-devel at mitgcm.org
> >> http://mitgcm.org/mailman/listinfo/mitgcm-devel
> >
> >
> > _______________________________________________
> > MITgcm-devel mailing list
> > MITgcm-devel at mitgcm.org
> > http://mitgcm.org/mailman/listinfo/mitgcm-devel
>
>
> _______________________________________________
> MITgcm-devel mailing list
> MITgcm-devel at mitgcm.org
> http://mitgcm.org/mailman/listinfo/mitgcm-devel
More information about the MITgcm-devel
mailing list