August 2012 - petsc-dev - mailman3.cels.anl.gov

[PATCH 1 of 3] superlu: fix spelling error 3.0 -> 3.1
by Sean Farley 26 Aug '12

26 Aug '12

# HG changeset patch # User Sean Farley <sean(a)mcs.anl.gov> # Date 1346018864 18000 # Node ID 56899038c8e3e8ee070c93767a77632405801842 # Parent 74d6d3408dc97996a15e1a94cf2e2112183ac7f8 superlu: fix spelling error 3.0 -> 3.1 diff --git a/config/PETSc/packages/SuperLU_DIST.py b/config/PETSc/packages/SuperLU_DIST.py --- a/config/PETSc/packages/SuperLU_DIST.py +++ b/config/PETSc/packages/SuperLU_DIST.py @@ -5,11 +5,11 @@ PETSc.package.NewPackage.__init__(self, framework) self.download = ['http://crd-legacy.lbl.gov/~xiaoye/SuperLU/superlu_dist_3.1.tar.gz', 'http://ftp.mcs.anl.gov/pub/petsc/externalpackages/superlu_dist_3.1.tar.gz'] self.functions = ['set_default_options_dist'] self.includes = ['superlu_ddefs.h'] - self.liblist = [['libsuperlu_dist_3.0.a']] + self.liblist = [['libsuperlu_dist_3.1.a']] self.requires32bitint = 0 self.complex = 1 # SuperLU_Dist does not work with --download-f-blas-lapack with Compaqf90 compiler on windows. # However it should work with intel ifort. self.worksonWindows = 1 @@ -26,11 +26,11 @@ def Install(self): import os g = open(os.path.join(self.packageDir,'make.inc'),'w') g.write('DSuperLUroot = '+self.packageDir+'\n') - g.write('DSUPERLULIB = $(DSuperLUroot)/libsuperlu_dist_3.0.'+self.setCompilers.AR_LIB_SUFFIX+'\n') + g.write('DSUPERLULIB = $(DSuperLUroot)/libsuperlu_dist_3.1.'+self.setCompilers.AR_LIB_SUFFIX+'\n') g.write('BLASDEF = -DUSE_VENDOR_BLAS\n') g.write('BLASLIB = '+self.libraries.toString(self.blasLapack.dlib)+'\n') g.write('IMPI = '+self.headers.toString(self.mpi.include)+'\n') g.write('MPILIB = '+self.libraries.toString(self.mpi.lib)+'\n') g.write('PMETISLIB = '+self.libraries.toString(self.parmetis.lib)+'\n')

1 0

[PATCH 0 of 3] Trying out mailing list patch review
by Sean Farley 26 Aug '12

26 Aug '12

Here are some patches that I would like reviewed before pushing. In particular, the cuda patch should work elsewhere but I don't have other non-mac gpu machines to test it on. I've kept these patches simple to help show how nice smaller chunks are to review :-)

1 0

Re: [petsc-dev] Patch review
by Hong Zhang 24 Aug '12

24 Aug '12

Cleaned all except 5. Pushed to petsc-dev. Hong On Fri, Aug 24, 2012 at 5:42 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > http://petsc.cs.iit.edu/petsc/petsc-dev/rev/067c5dbc352e > > Just a couple requests regarding this patch: > > 1. Please split independent functionality into separate patches. This does > three different things. > > 2. Please cut out extra whitespace like this and follow conventions about > not including args in the declarations. > > @@ -273,6 +274,12 @@ > PETSC_EXTERN PetscErrorCode KSPMonitorDefault(KSP,PetscInt,PetscReal,void > *); > PETSC_EXTERN PetscErrorCode > KSPLSQRMonitorDefault(KSP,PetscInt,PetscReal,void *); > PETSC_EXTERN PetscErrorCode KSPMonitorRange(KSP,PetscInt,PetscReal,void > *); > + > + > +PETSC_EXTERN PetscErrorCode KSPMonitorDynamicTolerance(KSP ksp,PetscInt > its,PetscReal fnorm,void *dummy); > +PETSC_EXTERN PetscErrorCode KSPMonitorDynamicToleranceDestroy(void > **dummy); > + > + > > 3. This doesn't look like an improvement: > > @@ -163,7 +163,7 @@ > /* scalar updates */ > omega = xi2 / xi3; > beta = - xi4 / sigma; > - rho = PetscSqrtReal(PetscAbsScalar(xi1 - omega * xi2)); /* residual > norm */ > + rho = sqrt(fabs(xi1 - omega * xi2)); /* residual norm */ > > /* vector updates */ > > > 4. Better to raise a PETSC_ERR_SUP than to just leave this fragment in a > comment > > + This code is only correct for the real case... Need to modify for > complex numbers... > > > 5. If this thing works (which putting it in the public interface usually > indicates) then it needs a man page. > > --- a/src/ksp/ksp/interface/iterativ.c > +++ b/src/ksp/ksp/interface/iterativ.c > @@ -330,6 +330,42 @@ > PetscFunctionReturn(0); > } > > +#undef __FUNCT__ > +#define __FUNCT__ "KSPMonitorDynamicTolerance" > +/* > + A hack to using dynamic tolerence in preconditioner > + */ > +PetscErrorCode KSPMonitorDynamicTolerance(KSP ksp,PetscInt its,PetscReal > fnorm,void *dummy) { > > > 6. Two different new features and no tests! > > > > I'm a little concerned that the current review process is all ex post > facto. It makes more work for reviewers and the history ends up looking > confusing. I think we should work out a review process that can happen > before things are pushed to petsc-dev, so that the authors of the patches > can clean them up in response to comments. When done right, this ends up > being less work for both the reviewers and the authors. >

1 0

Re: [petsc-dev] Patch review
by Sean Farley 24 Aug '12

24 Aug '12

On Fri, Aug 24, 2012 at 7:16 PM, Hong Zhang <hzhang(a)mcs.anl.gov> wrote: > Sorry, my sloppiness. > I should examining it before pushing. > I'll clean it, and will ask Jie to provide regression tests. Ok, but how should we review your cleaned up version before you push anything else?

1 0

Re: [petsc-dev] Patch review
by Jed Brown 24 Aug '12

24 Aug '12

On Fri, Aug 24, 2012 at 7:16 PM, Hong Zhang <hzhang(a)mcs.anl.gov> wrote: > Sorry, my sloppiness. > I should examining it before pushing. > I'll clean it, and will ask Jie to provide regression tests. > Sounds good, thanks. > > Hong > > On Fri, Aug 24, 2012 at 6:44 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > >> On Fri, Aug 24, 2012 at 6:38 PM, Jie Chen <jiechen(a)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.) >> > >

1 0

Re: [petsc-dev] Patch review
by Hong Zhang 24 Aug '12

24 Aug '12

Sorry, my sloppiness. I should examining it before pushing. I'll clean it, and will ask Jie to provide regression tests. Hong On Fri, Aug 24, 2012 at 6:44 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > On Fri, Aug 24, 2012 at 6:38 PM, Jie Chen <jiechen(a)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.) >

1 0

Re: [petsc-dev] Patch review
by Jed Brown 24 Aug '12

24 Aug '12

On Fri, Aug 24, 2012 at 6:38 PM, Jie Chen <jiechen(a)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.)

1 0

Re: [petsc-dev] Patch review
by Jie Chen 24 Aug '12

24 Aug '12

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. Jie On Aug 24, 2012, at 5:42 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > http://petsc.cs.iit.edu/petsc/petsc-dev/rev/067c5dbc352e > > Just a couple requests regarding this patch: > > 1. Please split independent functionality into separate patches. This does three different things. > > 2. Please cut out extra whitespace like this and follow conventions about not including args in the declarations. > > @@ -273,6 +274,12 @@ > PETSC_EXTERN PetscErrorCode KSPMonitorDefault(KSP,PetscInt,PetscReal,void *); > PETSC_EXTERN PetscErrorCode KSPLSQRMonitorDefault(KSP,PetscInt,PetscReal,void *); > PETSC_EXTERN PetscErrorCode KSPMonitorRange(KSP,PetscInt,PetscReal,void *); > + > + > +PETSC_EXTERN PetscErrorCode KSPMonitorDynamicTolerance(KSP ksp,PetscInt its,PetscReal fnorm,void *dummy); > +PETSC_EXTERN PetscErrorCode KSPMonitorDynamicToleranceDestroy(void **dummy); > + > + > > 3. This doesn't look like an improvement: > > @@ -163,7 +163,7 @@ > /* scalar updates */ > omega = xi2 / xi3; > beta = - xi4 / sigma; > - rho = PetscSqrtReal(PetscAbsScalar(xi1 - omega * xi2)); /* residual norm */ > + rho = sqrt(fabs(xi1 - omega * xi2)); /* residual norm */ > > /* vector updates */ > > > 4. Better to raise a PETSC_ERR_SUP than to just leave this fragment in a comment > > + This code is only correct for the real case... Need to modify for complex numbers... > > > 5. If this thing works (which putting it in the public interface usually indicates) then it needs a man page. > > --- a/src/ksp/ksp/interface/iterativ.c > +++ b/src/ksp/ksp/interface/iterativ.c > @@ -330,6 +330,42 @@ > PetscFunctionReturn(0); > } > > +#undef __FUNCT__ > +#define __FUNCT__ "KSPMonitorDynamicTolerance" > +/* > + A hack to using dynamic tolerence in preconditioner > + */ > +PetscErrorCode KSPMonitorDynamicTolerance(KSP ksp,PetscInt its,PetscReal fnorm,void *dummy) { > > > 6. Two different new features and no tests! > > > > I'm a little concerned that the current review process is all ex post facto. It makes more work for reviewers and the history ends up looking confusing. I think we should work out a review process that can happen before things are pushed to petsc-dev, so that the authors of the patches can clean them up in response to comments. When done right, this ends up being less work for both the reviewers and the authors.

1 0

Re: [petsc-dev] Patch review
by Sean Farley 24 Aug '12

24 Aug '12

> Just make sure it doesn't involve Sean holding the master password for the petsc repository and then getting hacked again. Touché. I would propose mailing list review so I don't have to leave emacs :-)

1 0

Re: [petsc-dev] Patch review
by Barry Smith 24 Aug '12

24 Aug '12

On Aug 24, 2012, at 5:42 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > > I'm a little concerned that the current review process is all ex post facto. It makes more work for reviewers and the history ends up looking confusing. I think we should work out a review process that can happen before things are pushed to petsc-dev, so that the authors of the patches can clean them up in response to comments. When done right, this ends up being less work for both the reviewers and the authors. Propose a system. Barry Just make sure it doesn't involve Sean holding the master password for the petsc repository and then getting hacked again.

1 0