[MITgcm-devel] naming for inAdMode switches

Martin Losch Martin.Losch at awi.de
Fri Nov 2 03:28:10 EDT 2012 

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

More information about the MITgcm-devel mailing list