[petsc-dev] Patch review

[Jie Chen]

Oh, in KSPSetTolerances(), the outer_rtol should be the inner_rtol instead... Could you push a fix for me please?

Jie



----- Original Message -----
From: "Jed Brown" <jedbrown at mcs.anl.gov>
To: "Jie Chen" <jiechen at mcs.anl.gov>
Cc: "For users of the development version of PETSc" <petsc-dev at mcs.anl.gov>, "Hong Zhang" <hzhang at mcs.anl.gov>
Sent: Tuesday, August 28, 2012 10:23:00 AM
Subject: Re: Patch review

I just pushed some fixes for compilation in complex mode 

http://petsc.cs.iit.edu/petsc/petsc-dev/rev/49030d6e26f7 

In the following code, surely you intended to pass inner_rtol to KSPSetTolerances()? 


PetscErrorCode KSPMonitorDynamicTolerance(KSP ksp,PetscInt its,PetscReal fnorm,void *dummy) { 
PetscErrorCode ierr; 
PetscReal *coef = (PetscReal*)dummy; 
PC pcksp; 
KSP kspinner; 
PetscReal outer_rtol, outer_abstol, outer_dtol, inner_rtol; 
PetscInt outer_maxits; 
PetscFunctionBegin; 
ierr = KSPGetPC(ksp,&pcksp);CHKERRQ(ierr); 
kspinner = NULL; 
ierr = PCKSPGetKSP(pcksp,&kspinner);CHKERRQ(ierr); 
if (kspinner) { 
ierr = KSPGetTolerances(ksp, &outer_rtol, &outer_abstol, &outer_dtol, &outer_maxits);CHKERRQ(ierr); 
inner_rtol = (*coef) * outer_rtol / fnorm; 
ierr = KSPSetTolerances(kspinner, outer_rtol, outer_abstol, outer_dtol, outer_maxits);CHKERRQ(ierr); 
} 
PetscFunctionReturn(0); 
} 



On Fri, Aug 24, 2012 at 6:44 PM, Jed Brown < jedbrown at mcs.anl.gov > wrote: 




On Fri, Aug 24, 2012 at 6:38 PM, Jie Chen < jiechen at mcs.anl.gov > wrote: 


I am in fact somewhat reluctant to push out everything before it is formally published and fully tested. The patch consists of more than one algorithmic developments that may be far away from maturity (but they are likely to work), though the authors might be the only ones who care about the new algorithms at this point. Besides, the codes are now full of hacks and lack documentation. I have no problem reverting petsc to the old version temporarily, and I promise I will clean everything to meet the production requirement, although this might not happen in a very short time. Meanwhile I think it also does not hurt to keep the patch as is, as the modification is likely to be used by the author circle only. 

There isn't a problem with experimental code, but if you are going to push experimental code, it should conform to the usual standards. No need to revert the patch unless you've already decided it is a failed experiment. 



If something is genuinely useful, we certainly like to have it in our bag of tricks immediately. Waiting until a paper is published to put it in the repo just means that it will take longer to find use in real applications. As far as I'm concerned, an ideal scenario is that by the time someone reads the paper, they already have the functionality in a released version of PETSc, thus can experiment on their own problems without even recompiling. (This being the lowest possible effort, it maximizes the chances of finding other applications where the method is useful, thus maximizing citations, in case that is the metric you care about.)