[petsc-dev] Updated material on creating merge requests and testing for PETSc

[Barry Smith]

  I have completed updating the material on creating merge requests, testing, 
and reviewing merge requests for PETSc that I presented on Thursday.

It can be found at 

https://gitlab.com/petsc/petsc/-/merge_requests/3356 
  and 
https://docs.petsc.org/en/barry-2020-10-14-docs-integration/developers/integration/
 


I have tried to incorporate everyone's feedbacks but there are sure to be 
missing items and mistakes.

Having this be complete, concise, correct, and clear is very important for all 
new developers as well as current developers so please take a look at it and 
help improve it. Think of all the pain each of you went through (I did as well) 
trying to scrap together your understanding of the process over the last year, 
hopefully we can remove most of the pain for future people.

  Thanks


  Barry

Re: [petsc-dev] Kokkos initialize

[Mark Adams]

I don't have a problem with that.

On Fri, Oct 16, 2020 at 11:16 AM Junchao Zhang 
wrote:

> Let me have a look.  cupminit.inc is a template for CUDA and HIP. It is OK
> if you see some symbols twice.
> --Junchao Zhang
>
>
> On Fri, Oct 16, 2020 at 8:22 AM Mark Adams  wrote:
>
>> Junchao, I see this in cupminit.inc (twice)
>>
>> #if defined(PETSC_HAVE_KOKKOS)
>> ierr = PetscKokkosInitialize_Private();CHKERRQ(ierr);
>> PetscBeganKokkos = PETSC_TRUE;
>> #endif
>>
>> And I see
>>
>> ierr = PetscKokkosInitializeCheck();CHKERRQ(ierr);
>>
>> In the Kokkos operators.
>>
>> Are these redundant?
>>
>> On Thu, Oct 15, 2020 at 10:44 PM Mark Adams  wrote:
>>
>>> Si it seems like these two calls in cupminit.inc are
>>> inconsistent with lazy:
>>>
>>> 22:41 adams/gamg-reduce-opt-cuda *= ~/petsc$ git grep PetscBeganKokkos
>>> src/sys/objects/cupminit.inc:PetscBeganKokkos = PETSC_TRUE;
>>> src/sys/objects/cupminit.inc:PetscBeganKokkos = PETSC_TRUE;
>>>
>>> I can do an MR to remove these if that is the case.
>>>
>>> Mark
>>>
>>> On Thu, Oct 15, 2020 at 8:34 PM Barry Smith  wrote:
>>>


   I thought the plan was that Kokkos also had a lazy initialization but
 perhaps it does not and needs to be fixed.

   Barry

 > On Oct 15, 2020, at 6:49 PM, Mark Adams  wrote:
 >
 > I am running a on SUMMIt with a Kokkos cuda configuration and while
 debugging with ddt I noticed that it spent a long time in KokkosInit, but I
 was not using Kokkos. KokkosInit was call in PETSc's GPU init, which seems
 logical enough, but it would be better if it is not called if you are not
 using Kokkos.
 >
 > I recall seeing places where Kokkos is checked when calling a Kokkos
 method (ie, lazy initialization). Do we have policy on whether we are being
 lazy with KokkosInit or not?
 >
 > Mark

Re: [petsc-dev] behavior of PetscDefined(PETSC_HAVE_CUDA)

[Jed Brown]

Patrick Sanan  writes:

> The man page for this seems to need fixing:
>
> "Either way evaluates true if PETSC_USE_DEBUG is defined (merely defined 
> or defined to 1) or undefined. This macro should not be used if its argument 
> may be defined to a non-empty value other than 1."
>
> Is a 0 value allowed? If so, does it also evaluate to false?

It would evaluate false, though

#define CONFIG_VALUE 0

is a cpp antipattern because

#ifdef CONFIG_VALUE
  this executes
#endif

>
> The source check seems easy enough - reject anything that matches the 
> following?
>
> PetscDefined(\W*PETSC_

Sure.

Re: [petsc-dev] Kokkos initialize

[Junchao Zhang]

Let me have a look.  cupminit.inc is a template for CUDA and HIP. It is OK
if you see some symbols twice.
--Junchao Zhang


On Fri, Oct 16, 2020 at 8:22 AM Mark Adams  wrote:

> Junchao, I see this in cupminit.inc (twice)
>
> #if defined(PETSC_HAVE_KOKKOS)
> ierr = PetscKokkosInitialize_Private();CHKERRQ(ierr);
> PetscBeganKokkos = PETSC_TRUE;
> #endif
>
> And I see
>
> ierr = PetscKokkosInitializeCheck();CHKERRQ(ierr);
>
> In the Kokkos operators.
>
> Are these redundant?
>
> On Thu, Oct 15, 2020 at 10:44 PM Mark Adams  wrote:
>
>> Si it seems like these two calls in cupminit.inc are
>> inconsistent with lazy:
>>
>> 22:41 adams/gamg-reduce-opt-cuda *= ~/petsc$ git grep PetscBeganKokkos
>> src/sys/objects/cupminit.inc:PetscBeganKokkos = PETSC_TRUE;
>> src/sys/objects/cupminit.inc:PetscBeganKokkos = PETSC_TRUE;
>>
>> I can do an MR to remove these if that is the case.
>>
>> Mark
>>
>> On Thu, Oct 15, 2020 at 8:34 PM Barry Smith  wrote:
>>
>>>
>>>
>>>   I thought the plan was that Kokkos also had a lazy initialization but
>>> perhaps it does not and needs to be fixed.
>>>
>>>   Barry
>>>
>>> > On Oct 15, 2020, at 6:49 PM, Mark Adams  wrote:
>>> >
>>> > I am running a on SUMMIt with a Kokkos cuda configuration and while
>>> debugging with ddt I noticed that it spent a long time in KokkosInit, but I
>>> was not using Kokkos. KokkosInit was call in PETSc's GPU init, which seems
>>> logical enough, but it would be better if it is not called if you are not
>>> using Kokkos.
>>> >
>>> > I recall seeing places where Kokkos is checked when calling a Kokkos
>>> method (ie, lazy initialization). Do we have policy on whether we are being
>>> lazy with KokkosInit or not?
>>> >
>>> > Mark
>>>
>>>

Re: [petsc-dev] behavior of PetscDefined(PETSC_HAVE_CUDA)

[Patrick Sanan]

The man page for this seems to need fixing:

"Either way evaluates true if PETSC_USE_DEBUG is defined (merely defined or 
defined to 1) or undefined. This macro should not be used if its argument may 
be defined to a non-empty value other than 1."

Is a 0 value allowed? If so, does it also evaluate to false?


The source check seems easy enough - reject anything that matches the following?

PetscDefined(\W*PETSC_

> Am 16.10.2020 um 14:59 schrieb Mark Adams :
> 
> 
> 
> On Fri, Oct 16, 2020 at 1:05 AM Jed Brown  wrote:
> Barry Smith  writes:
> 
> >   Could it error if the input starts with PETS_ to prevent inadvertent 
> > mistakes hanging around for years?
> 
> Source checks could warn, but cpp macros can't do substring matching.
> 
> You would want to check this before CI, so live with it. 

Re: [petsc-dev] Kokkos initialize

[Mark Adams]

Junchao, I see this in cupminit.inc (twice)

#if defined(PETSC_HAVE_KOKKOS)
ierr = PetscKokkosInitialize_Private();CHKERRQ(ierr);
PetscBeganKokkos = PETSC_TRUE;
#endif

And I see

ierr = PetscKokkosInitializeCheck();CHKERRQ(ierr);

In the Kokkos operators.

Are these redundant?

On Thu, Oct 15, 2020 at 10:44 PM Mark Adams  wrote:

> Si it seems like these two calls in cupminit.inc are
> inconsistent with lazy:
>
> 22:41 adams/gamg-reduce-opt-cuda *= ~/petsc$ git grep PetscBeganKokkos
> src/sys/objects/cupminit.inc:PetscBeganKokkos = PETSC_TRUE;
> src/sys/objects/cupminit.inc:PetscBeganKokkos = PETSC_TRUE;
>
> I can do an MR to remove these if that is the case.
>
> Mark
>
> On Thu, Oct 15, 2020 at 8:34 PM Barry Smith  wrote:
>
>>
>>
>>   I thought the plan was that Kokkos also had a lazy initialization but
>> perhaps it does not and needs to be fixed.
>>
>>   Barry
>>
>> > On Oct 15, 2020, at 6:49 PM, Mark Adams  wrote:
>> >
>> > I am running a on SUMMIt with a Kokkos cuda configuration and while
>> debugging with ddt I noticed that it spent a long time in KokkosInit, but I
>> was not using Kokkos. KokkosInit was call in PETSc's GPU init, which seems
>> logical enough, but it would be better if it is not called if you are not
>> using Kokkos.
>> >
>> > I recall seeing places where Kokkos is checked when calling a Kokkos
>> method (ie, lazy initialization). Do we have policy on whether we are being
>> lazy with KokkosInit or not?
>> >
>> > Mark
>>
>>

Re: [petsc-dev] behavior of PetscDefined(PETSC_HAVE_CUDA)

[Mark Adams]

On Fri, Oct 16, 2020 at 1:05 AM Jed Brown  wrote:

> Barry Smith  writes:
>
> >   Could it error if the input starts with PETS_ to prevent inadvertent
> mistakes hanging around for years?
>
> Source checks could warn, but cpp macros can't do substring matching.
>

You would want to check this before CI, so live with it.

Re: [petsc-dev] behavior of PetscDefined(PETSC_HAVE_CUDA)

[Mark Adams]

Ah, thanks.

On Thu, Oct 15, 2020 at 11:39 PM Jed Brown  wrote:

> Mark Adams  writes:
>
> > I have:
> >
> > #if defined(PETSC_HAVE_CUDA)
> >   ierr = PetscInfo3(pc,"PETSC_HAVE_CUDA -Test:
> > factor=%D. cuda=%D
> > level=%D\n",pc_gamg->level_reduction_factors[pc_gamg->current_level],
> > *PetscDefined(PETSC_HAVE_CUDA)*,pc_gamg->current_level);CHKERRQ(ierr);
> > #endif
> >
> > and see:
> >
> > [0] PCGAMGCreateLevel_GAMG(): PETSC_HAVE_CUDA -Test:
> > factor=2.* cuda=0* level=0
> >
> > So PETSC_HAVE_CUDA is defined yet PetscDefined(PETSC_HAVE_CUDA) is 0. Am
> I
> > missing something?
>
> The syntax is PetscDefined(HAVE_CUDA).  This is explained by way of
> example in the man page, but perhaps it should be explicitly repeated.
>
>
> https://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/Sys/PetscDefined.html
>