<meta http-equiv="Content-Type" content="text/html; charset=utf-8"><div dir="ltr"><div dir="ltr"><br><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 9, 2020 at 1:44 PM Lisandro Dalcin <<a href="mailto:dalcinl@gmail.com">dalcinl@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 9 Mar 2020 at 19:22, Junchao Zhang <<a href="mailto:jczhang@mcs.anl.gov" target="_blank">jczhang@mcs.anl.gov</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi, Lisandro,<div> It is very cool to see you can make petsc dance with slurm.</div></div></blockquote><div><br></div><div>Oh, boy... another poor soul lost in Dante's inferno...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div> From you pseudo example, my comments are:</div></div></blockquote><div><br></div><div>First of all, this is work in progress. I already have a few changes.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div>* Do we need a type PetscSigSet instead of explicit  int?</div></blockquote></div></blockquote><div><br></div><div>Maybe, not a big deal.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px">* Why do PetscSignalBegin/End() have different argument types? Many petsc XxxBegin/End() routines have the same arguments. It is easier to remember for users.<br></blockquote></div></blockquote><div><br></div><div>Right now I've change things this way:</div><div><br></div><div>PETSC_EXTERN PetscErrorCode PetscSignalBegin(int);<br>PETSC_EXTERN PetscErrorCode PetscSignalEnd(void);<br></div><div><br></div><div>In my current implementation, I have no use for passing a signal set to PetscSignalEnd().</div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>Maybe we should rename to PetscSignalPush(sigset) / PetscSignalPop() ?</div></div></div></blockquote><div>You can have PetscSignalEnd(int) without using the argument. Users are used to styles of PETSc's XxxBegin/End(), XxxGet/XxxRestore().</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px">* Why do you need PetscSigMask in public header?  </blockquote></div></blockquote><div><br></div><div>To construct a signal set to pass to Begin():</div><div><br></div><div>  mask  = 0;<br>  mask |= PetscSigMask(PETSC_SIGHUP);<br>  mask |= PetscSigMask(PETSC_SIGINT);<br>  mask |= PetscSigMask(PETSC_SIGCONT);<br>  mask |= PetscSigMask(PETSC_SIGTERM);<br>  mask |= PetscSigMask(PETSC_SIGUSR1);<br>  mask |= PetscSigMask(PETSC_SIGUSR2);<br>  ierr = PetscSignalBegin(mask);CHKERRQ(ierr);<br></div><div><br></div></div></div></blockquote>User can use your PetscSigSetAdd() to construct a SigSet and use it in PetscSignalBegin or PetscSignalClear<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px">Can user do PetscSignalClear(PETSC_SIGUSR1) instead of PetscSignalClear(PetscSigMask(PETSC_SIGUSR1))?</blockquote></div></blockquote><div><br></div><div>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. </div><div><br></div><div>Maybe we could have Remove(signum) and Clear(sigset) ?</div><div><br></div></div></div></blockquote><div>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.  </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div> I like fewer and simpler public APIs.  Just my two cents.</div><div><br></div></div></blockquote><div><br></div><div>Much appreciated!. Let me think a bit more about it, I agree that the current API is a bit weird...</div><div><br></div><div>PS: Do you  really think this may be actually useful in production to be worth to push into PETSc?</div></div></div></blockquote><div>If you find it is useful, then I believe someone else will also.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div></div><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Lisandro Dalcin<br>============<br>Research Scientist<br>Extreme Computing Research Center (ECRC)<br>King Abdullah University of Science and Technology (KAUST)<br><a href="http://ecrc.kaust.edu.sa/" target="_blank">http://ecrc.kaust.edu.sa/</a><br></div></div></div></div>
</blockquote></div></div>