February 2013 - petsc-dev - mailman3.cels.anl.gov

Re: [petsc-dev] (no subject)
by Jed Brown 16 Feb '13

16 Feb '13

On Sat, Feb 16, 2013 at 6:00 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > No, as I said in my previous email, it would be in > petsc-private/petscconfhypre.h > > When I wrote petsconfPACKAGE.h I meant this in the sense of > PETSc/packages/*.py NOT in the sense of sys, ksp, vec, etc. so there would > be a petsconfhypre.py petscconfml.py petscconfsuperlu.py etc etc etc > Ah, okay. > Yes there would also be a petscconfsys.h for things needed in the the sys > directory (just so they are not included in snes, ts, etc. > Just splitting petscconfsys.h and petscconfpackages.h might be enough (leaving petscconfbase.h or whatever to contain things like PETSC_USE_DEBUG) would remove dependencies from most source files (that care about neither packages nor sys). If we did this, we should at least put a comment /* #undef PETSC_HAVE_PACKAGE */ for those missing packages, so that we can easily grep to see which header the macro would be defined in. > No, I am not strongly advocating making this change. Just suggesting it > as a natural possibility related to the other recent changes > Yeah, I'm not convinced it's worth it.

1 0

Re: [petsc-dev] (no subject)
by Barry Smith 16 Feb '13

16 Feb '13

On Feb 16, 2013, at 5:53 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > > On Sat, Feb 16, 2013 at 5:43 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > There might be a petsconf.h petsc-private/petscconfimpl.h petsc-private/petscconfsys.h petsc-private/petscconfdraw.h petsc-privatre/petscconfPACKAGE.h > > Okay, but if we took this route, which of these would havePETSC_HAVE_HYPRE? Would it be petscconfksp.h because Hypre is a preconditioning library? Or petscconfdm.h because KSP depends on DM and there is some structured grid stuff in Hypre? Both of those answers would be wrong because of src/vec/vec/impls/hypre/vhyp.c, but I don't see any good way to locally reason about the link failures that would result from defining the macro at the wrong level. No, as I said in my previous email, it would be in petsc-private/petscconfhypre.h When I wrote petsconfPACKAGE.h I meant this in the sense of PETSc/packages/*.py NOT in the sense of sys, ksp, vec, etc. so there would be a petsconfhypre.py petscconfml.py petscconfsuperlu.py etc etc etc Yes there would also be a petscconfsys.h for things needed in the the sys directory (just so they are not included in snes, ts, etc. No, I am not strongly advocating making this change. Just suggesting it as a natural possibility related to the other recent changes Barry

1 0

Re: [petsc-dev] (no subject)
by Jed Brown 16 Feb '13

16 Feb '13

On Sat, Feb 16, 2013 at 5:43 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > There might be a petsconf.h petsc-private/petscconfimpl.h > petsc-private/petscconfsys.h petsc-private/petscconfdraw.h > petsc-privatre/petscconfPACKAGE.h > Okay, but if we took this route, which of these would have PETSC_HAVE_HYPRE? Would it be petscconfksp.h because Hypre is a preconditioning library? Or petscconfdm.h because KSP depends on DM and there is some structured grid stuff in Hypre? Both of those answers would be wrong because of src/vec/vec/impls/hypre/vhyp.c, but I don't see any good way to locally reason about the link failures that would result from defining the macro at the wrong level.

1 0

Re: [petsc-dev] (no subject)
by Barry Smith 16 Feb '13

16 Feb '13

On Feb 16, 2013, at 5:14 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > > On Sat, Feb 16, 2013 at 5:02 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > No one would "classify" PETSC_HAVE_HYPRE as being relevant to any particular thing. Rather if you are writing code anywhere in PETSc (Mat, PC, whatever) and wished to use hypre you would have > > #include <petsc-private/petscconfhypre.h> > > Ah, this is finer-grained than I had imagined, but may work. Reconfiguring with a new package would typically only require rebuilding a few files, which is nice. Plus there is already code to figure out the dependencies. > OTOH, this would create a zillion one-line files. Maybe only a few dozen. > > > #if defined(PETSC_HAVE_HYPE) > just like now > > Of course this requires always creating includes for all packages but maybe no biggie? > > I was concerned with how they would be classified. The majority of petscconf.h is machine/system stuff that changes rarely. There might be a petsconf.h petsc-private/petscconfimpl.h petsc-private/petscconfsys.h petsc-private/petscconfdraw.h petsc-privatre/petscconfPACKAGE.h

1 0

Re: [petsc-dev] (no subject)
by Jed Brown 16 Feb '13

16 Feb '13

On Sat, Feb 16, 2013 at 5:02 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > No one would "classify" PETSC_HAVE_HYPRE as being relevant to any > particular thing. Rather if you are writing code anywhere in PETSc (Mat, > PC, whatever) and wished to use hypre you would have > > #include <petsc-private/petscconfhypre.h> > Ah, this is finer-grained than I had imagined, but may work. Reconfiguring with a new package would typically only require rebuilding a few files, which is nice. OTOH, this would create a zillion one-line files. > > #if defined(PETSC_HAVE_HYPE) > just like now > > Of course this requires always creating includes for all packages but > maybe no biggie? > I was concerned with how they would be classified. The majority of petscconf.h is machine/system stuff that changes rarely.

1 0

Re: [petsc-dev] (no subject)
by Barry Smith 16 Feb '13

16 Feb '13

On Feb 16, 2013, at 8:48 AM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > > On Sat, Feb 16, 2013 at 5:53 AM, Matthew Knepley <knepley(a)gmail.com> wrote: > > Splitting petscconf.h by package or something would be cool because when reconfigure runs, any changes to petscconf.h usually don't affect anything, but I think trying to do that would be a maintenance disaster. > > Might not be too bad actually. If the Package class itself just handled it as a matter of course, declaring Info with a package (and when called from within a package) would automatically to into its own list and then its own file. > > The file that it outputs into is set by the object, so all you need to do is reset the header filename in the > constructor for Package. > > The reason I thought it was hazardous was if, say, someone classified PETSC_HAVE_HYPRE as being relevant to KSP/PC, then anyone testing for it from code in petscmat would come up false for no apparent reason. No one would "classify" PETSC_HAVE_HYPRE as being relevant to any particular thing. Rather if you are writing code anywhere in PETSc (Mat, PC, whatever) and wished to use hypre you would have #include <petsc-private/petscconfhypre.h> #if defined(PETSC_HAVE_HYPE) just like now Of course this requires always creating includes for all packages but maybe no biggie? Barry > To figure out why it's false, we would have to look through the petscconfxxx.h files, which is just more confusing. > > An alternative is to #define PETSC_HAVE_X 0 instead of leaving it undefined, so that we can get a compilation error when we haven't included the right conf header. This would convert a lot of #if defined(X) ... #endif code to non-cpp if (X) { ... }. > > Note that we could #define PETSC_HAVE_X_UNDEFINED 1 and write a PETSC_HAVE_(X) macro that errors if neither PETSC_HAVE_X or PETSC_HAVE_X_UNDEFINED is available, thus tests based on defined state would still be valid, but we could alert the user when they need to reconfigure. > > I just don't know if any of this is worth it.

1 0

Re: [petsc-dev] (no subject)
by Jed Brown 16 Feb '13

16 Feb '13

On Sat, Feb 16, 2013 at 5:53 AM, Matthew Knepley <knepley(a)gmail.com> wrote: > > Splitting petscconf.h by package or something would be cool because when >> reconfigure runs, any changes to petscconf.h usually don't affect anything, >> but I think trying to do that would be a maintenance disaster. >> >> Might not be too bad actually. If the Package class itself just >> handled it as a matter of course, declaring Info with a package (and when >> called from within a package) would automatically to into its own list and >> then its own file. > > > The file that it outputs into is set by the object, so all you need to do > is reset the header filename in the > constructor for Package. > The reason I thought it was hazardous was if, say, someone classified PETSC_HAVE_HYPRE as being relevant to KSP/PC, then anyone testing for it from code in petscmat would come up false for no apparent reason. To figure out why it's false, we would have to look through the petscconfxxx.h files, which is just more confusing. An alternative is to #define PETSC_HAVE_X 0 instead of leaving it undefined, so that we can get a compilation error when we haven't included the right conf header. This would convert a lot of #if defined(X) ... #endif code to non-cpp if (X) { ... }. Note that we could #define PETSC_HAVE_X_UNDEFINED 1 and write a PETSC_HAVE_(X) macro that errors if neither PETSC_HAVE_X or PETSC_HAVE_X_UNDEFINED is available, thus tests based on defined state would still be valid, but we could alert the user when they need to reconfigure. I just don't know if any of this is worth it.

1 0

Re: [petsc-dev] (no subject)
by Matthew Knepley 16 Feb '13

16 Feb '13

On Fri, Feb 15, 2013 at 10:02 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > > On Feb 15, 2013, at 8:56 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > > > On Fri, Feb 15, 2013 at 8:33 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > > > > We should probably have a petscconf.h and a > petsc-private/petscconfimpl.h > > > > What would that do? > > Now everything that is figured out is visible in user code. Yes it is > name spaced but 80+% of it is likely never relevant for user code and if > the rule is don't expose stuff that is not relevant it does violate that :-) > > > > I think splitting petscsys.h (plus transitive includes) may be the best > value. petscsys.h itself has a lot of comments (man pages for each numbered > PetscMallocN, ...), but so many of its functions are needed by every caller > that it may not provide much value to split it. Of course variadic macros > would drastically consolidate that (and the error handling). > > > > Splitting petscconf.h by package or something would be cool because when > reconfigure runs, any changes to petscconf.h usually don't affect anything, > but I think trying to do that would be a maintenance disaster. > > Might not be too bad actually. If the Package class itself just handled > it as a matter of course, declaring Info with a package (and when called > from within a package) would automatically to into its own list and then > its own file. The file that it outputs into is set by the object, so all you need to do is reset the header filename in the constructor for Package. Matt > > Barry > > > > > > > wc -l include/*.h include/petsc-private/*.h arch-gnu/include/petscconf.h > | sort > > 102 include/petsccharacteristic.h > > 113 include/petscbt.h > > 114 include/petsc-private/petscaxpy.h > > 122 include/petscblaslapack.h > > 124 include/petsc-private/logimpl.h > > 131 include/petsc-private/petscfptimpl.h > > 140 include/petscctable.h > > 176 include/petsctime.h > > 181 include/petsc-private/threadcommimpl.h > > 182 include/petscblaslapack_c.h > > 182 include/petscblaslapack_caps.h > > 185 include/petscdmplex.h > > 193 include/petscsf.h > > 218 include/petsc-private/dmimpl.h > > 219 include/petsc-private/tsimpl.h > > 223 include/petsc-private/fortranimpl.h > > 242 include/petsc-private/snesimpl.h > > 250 include/petscoptions.h > > 256 include/petscdm.h > > 262 include/petscblaslapack_uscore.h > > 266 include/petscdmda.h > > 276 include/petsc-private/kspimpl.h > > 282 include/petscblaslapack_stdcall.h > > 304 include/petscdmmesh.h > > 314 include/petscdraw.h > > 334 include/petsc-private/vecimpl.h > > 451 include/petscmath.h > > 455 include/petscviewer.h > > 461 include/petscis.h > > 533 include/petsclog.h > > 564 include/petscvec.h > > 596 include/petscpc.h > > 649 include/petscksp.h > > 741 include/petscsnes.h > > 754 include/petscerror.h > > 845 include/petscts.h > > 858 include/petsc-private/petscimpl.h > > 1036 arch-gnu/include/petscconf.h > > 1504 include/petsc-private/matimpl.h > > 1930 include/petscmat.h > > 2399 include/petscsys.h > > 20713 total > > > > > > > > -- What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead. -- Norbert Wiener

1 0

Re: [petsc-dev] (no subject)
by Barry Smith 15 Feb '13

15 Feb '13

On Feb 15, 2013, at 8:56 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > On Fri, Feb 15, 2013 at 8:33 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > > We should probably have a petscconf.h and a petsc-private/petscconfimpl.h > > What would that do? Now everything that is figured out is visible in user code. Yes it is name spaced but 80+% of it is likely never relevant for user code and if the rule is don't expose stuff that is not relevant it does violate that :-) > > I think splitting petscsys.h (plus transitive includes) may be the best value. petscsys.h itself has a lot of comments (man pages for each numbered PetscMallocN, ...), but so many of its functions are needed by every caller that it may not provide much value to split it. Of course variadic macros would drastically consolidate that (and the error handling). > > Splitting petscconf.h by package or something would be cool because when reconfigure runs, any changes to petscconf.h usually don't affect anything, but I think trying to do that would be a maintenance disaster. Might not be too bad actually. If the Package class itself just handled it as a matter of course, declaring Info with a package (and when called from within a package) would automatically to into its own list and then its own file. Barry > > > wc -l include/*.h include/petsc-private/*.h arch-gnu/include/petscconf.h | sort > 102 include/petsccharacteristic.h > 113 include/petscbt.h > 114 include/petsc-private/petscaxpy.h > 122 include/petscblaslapack.h > 124 include/petsc-private/logimpl.h > 131 include/petsc-private/petscfptimpl.h > 140 include/petscctable.h > 176 include/petsctime.h > 181 include/petsc-private/threadcommimpl.h > 182 include/petscblaslapack_c.h > 182 include/petscblaslapack_caps.h > 185 include/petscdmplex.h > 193 include/petscsf.h > 218 include/petsc-private/dmimpl.h > 219 include/petsc-private/tsimpl.h > 223 include/petsc-private/fortranimpl.h > 242 include/petsc-private/snesimpl.h > 250 include/petscoptions.h > 256 include/petscdm.h > 262 include/petscblaslapack_uscore.h > 266 include/petscdmda.h > 276 include/petsc-private/kspimpl.h > 282 include/petscblaslapack_stdcall.h > 304 include/petscdmmesh.h > 314 include/petscdraw.h > 334 include/petsc-private/vecimpl.h > 451 include/petscmath.h > 455 include/petscviewer.h > 461 include/petscis.h > 533 include/petsclog.h > 564 include/petscvec.h > 596 include/petscpc.h > 649 include/petscksp.h > 741 include/petscsnes.h > 754 include/petscerror.h > 845 include/petscts.h > 858 include/petsc-private/petscimpl.h > 1036 arch-gnu/include/petscconf.h > 1504 include/petsc-private/matimpl.h > 1930 include/petscmat.h > 2399 include/petscsys.h > 20713 total > > >

1 0

Re: [petsc-dev] (no subject)
by Jed Brown 15 Feb '13

15 Feb '13

On Fri, Feb 15, 2013 at 8:33 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > > We should probably have a petscconf.h and a petsc-private/petscconfimpl.h > What would that do? I think splitting petscsys.h (plus transitive includes) may be the best value. petscsys.h itself has a lot of comments (man pages for each numbered PetscMallocN, ...), but so many of its functions are needed by every caller that it may not provide much value to split it. Of course variadic macros would drastically consolidate that (and the error handling). Splitting petscconf.h by package or something would be cool because when reconfigure runs, any changes to petscconf.h usually don't affect anything, but I think trying to do that would be a maintenance disaster. > > wc -l include/*.h include/petsc-private/*.h arch-gnu/include/petscconf.h | > sort > 102 include/petsccharacteristic.h > 113 include/petscbt.h > 114 include/petsc-private/petscaxpy.h > 122 include/petscblaslapack.h > 124 include/petsc-private/logimpl.h > 131 include/petsc-private/petscfptimpl.h > 140 include/petscctable.h > 176 include/petsctime.h > 181 include/petsc-private/threadcommimpl.h > 182 include/petscblaslapack_c.h > 182 include/petscblaslapack_caps.h > 185 include/petscdmplex.h > 193 include/petscsf.h > 218 include/petsc-private/dmimpl.h > 219 include/petsc-private/tsimpl.h > 223 include/petsc-private/fortranimpl.h > 242 include/petsc-private/snesimpl.h > 250 include/petscoptions.h > 256 include/petscdm.h > 262 include/petscblaslapack_uscore.h > 266 include/petscdmda.h > 276 include/petsc-private/kspimpl.h > 282 include/petscblaslapack_stdcall.h > 304 include/petscdmmesh.h > 314 include/petscdraw.h > 334 include/petsc-private/vecimpl.h > 451 include/petscmath.h > 455 include/petscviewer.h > 461 include/petscis.h > 533 include/petsclog.h > 564 include/petscvec.h > 596 include/petscpc.h > 649 include/petscksp.h > 741 include/petscsnes.h > 754 include/petscerror.h > 845 include/petscts.h > 858 include/petsc-private/petscimpl.h > 1036 arch-gnu/include/petscconf.h > 1504 include/petsc-private/matimpl.h > 1930 include/petscmat.h > 2399 include/petscsys.h > 20713 total > > >

1 0