We are in agreement.  I should not use double negatives.

This change should be made.

Todd.

> On Aug 21, 2016, at 11:39 PM, Barry Smith <[email protected]> wrote:
> 
> 
>> On Aug 21, 2016, at 11:33 PM, Munson, Todd <[email protected]> wrote:
>> 
>> 
>> I don't see any reason not to make the change.
> 
>   I any new solvers are to be added in the future that utilize the KSPCGXXX 
> solver functionality they would be easier and cleaner to implement.
> 
>   Barry
> 
>> 
>> Todd.
>> 
>>> On Aug 21, 2016, at 9:40 PM, Barry Smith <[email protected]> wrote:
>>> 
>>> 
>>> Todd,
>>> 
>>>  The TAO solvers that utilize KSPNASH STCG or GLTR have all this horribly 
>>> redundant unneeded code 
>>> 
>>>    /* Solve the trust region subproblem */
>>>    if (NTR_KSP_NASH == tr->ksp_type) {
>>>      ierr = KSPNASHSetRadius(tao->ksp,tao->trust);CHKERRQ(ierr);
>>>      ierr = KSPSolve(tao->ksp, tao->gradient, 
>>> tao->stepdirection);CHKERRQ(ierr);
>>>      ierr = KSPGetIterationNumber(tao->ksp,&its);CHKERRQ(ierr);
>>>      tao->ksp_its+=its;
>>>      tao->ksp_tot_its+=its;
>>>      ierr = KSPNASHGetNormD(tao->ksp, &norm_d);CHKERRQ(ierr);
>>>    } else if (NTR_KSP_STCG == tr->ksp_type) {
>>>      ierr = KSPSTCGSetRadius(tao->ksp,tao->trust);CHKERRQ(ierr);
>>>      ierr = KSPSolve(tao->ksp, tao->gradient, 
>>> tao->stepdirection);CHKERRQ(ierr);
>>>      ierr = KSPGetIterationNumber(tao->ksp,&its);CHKERRQ(ierr);
>>>      tao->ksp_its+=its;
>>>      tao->ksp_tot_its+=its;
>>>      ierr = KSPSTCGGetNormD(tao->ksp, &norm_d);CHKERRQ(ierr);
>>>    } else { /* NTR_KSP_GLTR */
>>>      ierr = KSPGLTRSetRadius(tao->ksp,tao->trust);CHKERRQ(ierr);
>>>      ierr = KSPSolve(tao->ksp, tao->gradient, 
>>> tao->stepdirection);CHKERRQ(ierr);
>>>      ierr = KSPGetIterationNumber(tao->ksp,&its);CHKERRQ(ierr);
>>>      tao->ksp_its+=its;
>>>      tao->ksp_tot_its+=its;
>>>      ierr = KSPGLTRGetNormD(tao->ksp, &norm_d);CHKERRQ(ierr);
>>>    }
>>> 
>>> This can be simplified a great deal by making the names 
>>> 
>>> KSPCGSetRadius(), KSPGGGetNormD() etc you only need to change the 
>>> registration from 
>>> 
>>> ierr = 
>>> PetscObjectComposeFunction((PetscObject)ksp,"KSPNASHSetRadius_C",KSPNASHSetRadius_NASH);CHKERRQ(ierr);
>>> ierr = 
>>> PetscObjectComposeFunction((PetscObject)ksp,"KSPNASHGetNormD_C",KSPNASHGetNormD_NASH);CHKERRQ(ierr);
>>> ierr = 
>>> PetscObjectComposeFunction((PetscObject)ksp,"KSPNASHGetObjFcn_C",KSPNASHGetObjFcn_NASH);CHKERRQ(ierr);
>>> 
>>> to 
>>> 
>>> 
>>> ierr = 
>>> PetscObjectComposeFunction((PetscObject)ksp,"KSPCGSetRadius_C",KSPCGSetRadius_NASH);CHKERRQ(ierr);
>>> ierr = 
>>> PetscObjectComposeFunction((PetscObject)ksp,"KSPCGGetNormD_C",KSPCGGetNormD_NASH);CHKERRQ(ierr);
>>> ierr = 
>>> PetscObjectComposeFunction((PetscObject)ksp,"KSPCGGetObjFcn_C",KSPCGGetObjFcn_NASH);CHKERRQ(ierr);
>>> 
>>> Is there any reason we shouldn't do this? Essentially these three KSPs are 
>>> a special subclass of KSPCG that supports these additional methods.  
>>> Anything specific to an individual one like the Lanczo stuff still remains 
>>> with it.
>>> 
>>> Barry
>>> 
>>> 
>>> 
>>> 
>> 
> 

Reply via email to