I will start making this simplification today in a new branch. Todd.
> On Aug 22, 2016, at 6:18 AM, Munson, Todd <[email protected]> wrote: > > > 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 >>>> >>>> >>>> >>>> >>> >> >
