On Mon, 9 Mar 2020 at 19:22, Junchao Zhang <jczh...@mcs.anl.gov> 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() ? > * 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); > 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) ? > 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? -- Lisandro Dalcin ============ Research Scientist Extreme Computing Research Center (ECRC) King Abdullah University of Science and Technology (KAUST) http://ecrc.kaust.edu.sa/