On Mon, Mar 9, 2020 at 1:44 PM Lisandro Dalcin <[email protected]> wrote:
> > > On Mon, 9 Mar 2020 at 19:22, Junchao Zhang <[email protected]> wrote: > >> Hi, Lisandro, >> It is very cool to see you can make petsc dance with slurm. >> > > Oh, boy... another poor soul lost in Dante's inferno... > > >> From you pseudo example, my comments are: >> > > First of all, this is work in progress. I already have a few changes. > > >> * Do we need a type PetscSigSet instead of explicit int? >> >> > Maybe, not a big deal. > > >> * Why do PetscSignalBegin/End() have different argument types? Many petsc >> XxxBegin/End() routines have the same arguments. It is easier to remember >> for users. >> >> > Right now I've change things this way: > > PETSC_EXTERN PetscErrorCode PetscSignalBegin(int); > PETSC_EXTERN PetscErrorCode PetscSignalEnd(void); > > In my current implementation, I have no use for passing a signal set > to PetscSignalEnd(). > > Maybe we should rename to PetscSignalPush(sigset) / PetscSignalPop() ? > You can have PetscSignalEnd(int) without using the argument. Users are used to styles of PETSc's XxxBegin/End(), XxxGet/XxxRestore(). > > >> * Why do you need PetscSigMask in public header? >> >> > To construct a signal set to pass to Begin(): > > mask = 0; > mask |= PetscSigMask(PETSC_SIGHUP); > mask |= PetscSigMask(PETSC_SIGINT); > mask |= PetscSigMask(PETSC_SIGCONT); > mask |= PetscSigMask(PETSC_SIGTERM); > mask |= PetscSigMask(PETSC_SIGUSR1); > mask |= PetscSigMask(PETSC_SIGUSR2); > ierr = PetscSignalBegin(mask);CHKERRQ(ierr); > > User can use your PetscSigSetAdd() to construct a SigSet and use it in PetscSignalBegin or PetscSignalClear > > >> Can user do PetscSignalClear(PETSC_SIGUSR1) instead of >> PetscSignalClear(PetscSigMask(PETSC_SIGUSR1))? >> >> > But then if you need to clear many signals, you have to call multiple > times. Note that all my APIs PetscSignalXXX work with bitsets, not > individual signals. > > Maybe we could have Remove(signum) and Clear(sigset) ? > > You can keep the "all PetscSignalXxx arguments are SigSet" design. If users pass a signal to them, you can detect this error, for example, by using a 64-bit SigSet and leaving the lower 5 bits unused and starting the first bitmask at 6-th bit. > >> I like fewer and simpler public APIs. Just my two cents. >> >> > Much appreciated!. Let me think a bit more about it, I agree that the > current API is a bit weird... > > PS: Do you really think this may be actually useful in production to be > worth to push into PETSc? > If you find it is useful, then I believe someone else will also. > > > -- > Lisandro Dalcin > ============ > Research Scientist > Extreme Computing Research Center (ECRC) > King Abdullah University of Science and Technology (KAUST) > http://ecrc.kaust.edu.sa/ >
