Re: [petsc-dev] Wrong "failed tests" command

2019-10-21 Thread Jed Brown via petsc-dev
Switch to search (we really don't need to invoke Python for this) and slap a % 
on the end of anything where there might be extra.

make -f gmakefile test globsearch='mat_tests-ex128_1 
mat_tests-ex37_nsize-2_mat_type-mpibaij_mat_block_size%'

Scott Kruger  writes:

> When we created the map of directory+test+variants to targets we did not 
> use enough delimiters to allow the inverse map to be determined so 
> creating the proper list of target names is an ill-posed problem.   The 
> globsearch was a way of trying to just catch more of them, but obviously 
> it still has a bug.  I'll look at it.
>
> Scott
>
>
> On 10/21/19 12:47 PM, Jed Brown wrote:
>> Yeah, it's missing the numeric block size.  The following works
>> 
>> /usr/bin/make -f gmakefile test globsearch='mat_tests-ex128_1 
>> mat_tests-ex37_nsize-2_mat_type-mpibaij_mat_block_size-1'
>> 
>> Also, globsearch can be replaced by search in this usage.
>> 
>> "Smith, Barry F. via petsc-dev"  writes:
>> 
>>>May need more work on the tester infrastructure?
>>>
 On Oct 21, 2019, at 12:30 PM, Pierre Jolivet via petsc-dev 
  wrote:

 Hello,
 In this pipeline build log, 
 https://gitlab.com/petsc/petsc/-/jobs/326525063, it shows that I can rerun 
 failed tests using the following command:
 /usr/bin/make -f gmakefile test globsearch='mat_tests-ex128_1 
 mat_tests-ex37_nsize-2_mat_type-mpibaij_mat_block_size 
 mat_tests-ex37_nsize-1_mat_type-mpibaij_mat_block_size mat_tests-ex128_2 
 mat_tests-ex37_nsize-2_mat_type-baij_mat_block_size 
 mat_tests-ex37_nsize-1_mat_type-baij_mat_block_size mat_tests-ex30_4 
 mat_tests-ex37_nsize-2_mat_type-sbaij_mat_block_size mat_tests-ex18_* 
 mat_tests-ex76_3 mat_tests-ex37_nsize-2_mat_type-mpisbaij_mat_block_size 
 mat_tests-ex37_nsize-1_mat_type-sbaij_mat_block_size’

 If used, this command does not run any of the mat_test-ex37* tests.

 Thanks,
 Pierre
>
> -- 
> Tech-X Corporation   kru...@txcorp.com
> 5621 Arapahoe Ave, Suite A   Phone: (720) 974-1841
> Boulder, CO 80303Fax:   (303) 448-7756


Re: [petsc-dev] Wrong "failed tests" command

2019-10-21 Thread Scott Kruger via petsc-dev




When we created the map of directory+test+variants to targets we did not 
use enough delimiters to allow the inverse map to be determined so 
creating the proper list of target names is an ill-posed problem.   The 
globsearch was a way of trying to just catch more of them, but obviously 
it still has a bug.  I'll look at it.


Scott


On 10/21/19 12:47 PM, Jed Brown wrote:

Yeah, it's missing the numeric block size.  The following works

/usr/bin/make -f gmakefile test globsearch='mat_tests-ex128_1 
mat_tests-ex37_nsize-2_mat_type-mpibaij_mat_block_size-1'

Also, globsearch can be replaced by search in this usage.

"Smith, Barry F. via petsc-dev"  writes:


   May need more work on the tester infrastructure?


On Oct 21, 2019, at 12:30 PM, Pierre Jolivet via petsc-dev 
 wrote:

Hello,
In this pipeline build log, https://gitlab.com/petsc/petsc/-/jobs/326525063, it 
shows that I can rerun failed tests using the following command:
/usr/bin/make -f gmakefile test globsearch='mat_tests-ex128_1 
mat_tests-ex37_nsize-2_mat_type-mpibaij_mat_block_size 
mat_tests-ex37_nsize-1_mat_type-mpibaij_mat_block_size mat_tests-ex128_2 
mat_tests-ex37_nsize-2_mat_type-baij_mat_block_size 
mat_tests-ex37_nsize-1_mat_type-baij_mat_block_size mat_tests-ex30_4 
mat_tests-ex37_nsize-2_mat_type-sbaij_mat_block_size mat_tests-ex18_* 
mat_tests-ex76_3 mat_tests-ex37_nsize-2_mat_type-mpisbaij_mat_block_size 
mat_tests-ex37_nsize-1_mat_type-sbaij_mat_block_size’

If used, this command does not run any of the mat_test-ex37* tests.

Thanks,
Pierre


--
Tech-X Corporation   kru...@txcorp.com
5621 Arapahoe Ave, Suite A   Phone: (720) 974-1841
Boulder, CO 80303Fax:   (303) 448-7756


Re: [petsc-dev] Wrong "failed tests" command

2019-10-21 Thread Jed Brown via petsc-dev
Yeah, it's missing the numeric block size.  The following works

/usr/bin/make -f gmakefile test globsearch='mat_tests-ex128_1 
mat_tests-ex37_nsize-2_mat_type-mpibaij_mat_block_size-1'

Also, globsearch can be replaced by search in this usage.

"Smith, Barry F. via petsc-dev"  writes:

>   May need more work on the tester infrastructure?
>
>> On Oct 21, 2019, at 12:30 PM, Pierre Jolivet via petsc-dev 
>>  wrote:
>> 
>> Hello,
>> In this pipeline build log, https://gitlab.com/petsc/petsc/-/jobs/326525063, 
>> it shows that I can rerun failed tests using the following command:
>> /usr/bin/make -f gmakefile test globsearch='mat_tests-ex128_1 
>> mat_tests-ex37_nsize-2_mat_type-mpibaij_mat_block_size 
>> mat_tests-ex37_nsize-1_mat_type-mpibaij_mat_block_size mat_tests-ex128_2 
>> mat_tests-ex37_nsize-2_mat_type-baij_mat_block_size 
>> mat_tests-ex37_nsize-1_mat_type-baij_mat_block_size mat_tests-ex30_4 
>> mat_tests-ex37_nsize-2_mat_type-sbaij_mat_block_size mat_tests-ex18_* 
>> mat_tests-ex76_3 mat_tests-ex37_nsize-2_mat_type-mpisbaij_mat_block_size 
>> mat_tests-ex37_nsize-1_mat_type-sbaij_mat_block_size’
>> 
>> If used, this command does not run any of the mat_test-ex37* tests.
>> 
>> Thanks,
>> Pierre


Re: [petsc-dev] Wrong "failed tests" command

2019-10-21 Thread Smith, Barry F. via petsc-dev

  May need more work on the tester infrastructure?

> On Oct 21, 2019, at 12:30 PM, Pierre Jolivet via petsc-dev 
>  wrote:
> 
> Hello,
> In this pipeline build log, https://gitlab.com/petsc/petsc/-/jobs/326525063, 
> it shows that I can rerun failed tests using the following command:
> /usr/bin/make -f gmakefile test globsearch='mat_tests-ex128_1 
> mat_tests-ex37_nsize-2_mat_type-mpibaij_mat_block_size 
> mat_tests-ex37_nsize-1_mat_type-mpibaij_mat_block_size mat_tests-ex128_2 
> mat_tests-ex37_nsize-2_mat_type-baij_mat_block_size 
> mat_tests-ex37_nsize-1_mat_type-baij_mat_block_size mat_tests-ex30_4 
> mat_tests-ex37_nsize-2_mat_type-sbaij_mat_block_size mat_tests-ex18_* 
> mat_tests-ex76_3 mat_tests-ex37_nsize-2_mat_type-mpisbaij_mat_block_size 
> mat_tests-ex37_nsize-1_mat_type-sbaij_mat_block_size’
> 
> If used, this command does not run any of the mat_test-ex37* tests.
> 
> Thanks,
> Pierre



Re: [petsc-dev] "participants" on gitlab

2019-10-21 Thread Balay, Satish via petsc-dev
On Mon, 21 Oct 2019, Zhang, Hong via petsc-dev wrote:

> How is the list of participants determined when a MR is created on gitlab? It 
> seems to include everybody by default. Is there any way to shorten the list? 
> Ideally only the participants involved in the particular MR should be picked. 
> Note that currently there is a huge gap between the ''Participate'' and ''On 
> mention'' levels in the notification settings. With the former, I get spammed 
> with notifications whenever a new MR is created. With the later, I won’t 
> receive any notification (even someone replied my comments) unless explicitly 
> @ by someone.
> 
> Hong (Mr.)

Copy/pasting some prior notes from Scott and Alp on this..

Satish

---

On Mon, 30 Sep 2019, Dener, Alp wrote:
>
> It isn’t doing what it should be, but I’m keeping it at “Participate” to make 
> sure I don’t miss any MRs that I need to be looking at.
>
> In the meantime, there’s an active ongoing discussion about this notification 
> issue here: https://gitlab.com/gitlab-org/gitlab/issues/4816
>
>
> On September 26, 2019 at 11:37:18 AM, Alp Dener (ade...@anl.gov) wrote:
>
> I switched it to “Participate” for the developer group. I previously had my 
> global setting at “participate” and developer set to “global” and that didn’t 
> seem to work, but maybe there’s
> a bug that requires developer to be specifically set. Hopefully this’ll work.


On Thu, 12 Sep 2019, Scott Kruger wrote:
>
> Here's what I did:
>
> Settings -> Notifications -> developers + Participate
>
> The default is "Global". "Participate" is what I'm using now.
>
> There is a "Custom", but it confuses me since it says you can
> use it to match "Participate", but you can't do something like:
> Email all new issues, but only show me the MR's I am mentioned
> in or own.



Re: [petsc-dev] "participants" on gitlab

2019-10-21 Thread Smith, Barry F. via petsc-dev


> On Oct 21, 2019, at 10:53 AM, Fande Kong  wrote:
> 
> I have these troubles as well. I know nothing about gitlab.
> 
> It would be great if we could have the same setting as we had in bitbucket 
> before.

   I don't think there is a simple mapping.


> 
> Fande.
> 
> On Mon, Oct 21, 2019 at 9:47 AM Smith, Barry F. via petsc-dev 
>  wrote:
> 
> 
> > On Oct 21, 2019, at 10:27 AM, Zhang, Hong via petsc-dev 
> >  wrote:
> > 
> > How is the list of participants determined when a MR is created on gitlab? 
> > It seems to include everybody by default. Is there any way to shorten the 
> > list? Ideally only the participants involved in the particular MR should be 
> > picked. Note that currently there is a huge gap between the ''Participate'' 
> > and ''On mention'' levels in the notification settings. With the former, I 
> > get spammed with notifications whenever a new MR is created. With the 
> > later, I won’t receive any notification (even someone replied my comments) 
> > unless explicitly @ by someone.
> 
>   If you make a MR and then on the right side slide over Notifications button 
> do you not get all mail on the MR even if you don't do the Participate?  It 
> seems for me it is slide over automatically.
> 
>   We don't know GitLab any better than you so you could also google to find 
> out how to handle this. I now have a bunch of mail filters that help me from 
> getting useless crap about MR but still get more perhaps than I want. You may 
> need to add mail filters also.
> 
> 
>Barry
> 
> 
> 
> > 
> > Hong (Mr.)
> 



Re: [petsc-dev] ksp_error_if_not_converged in multilevel solvers

2019-10-21 Thread Smith, Barry F. via petsc-dev


> On Oct 21, 2019, at 12:55 AM, Pierre Jolivet  
> wrote:
> 
> 
> 
> On Oct 20, 2019, at 6:07 PM, "Smith, Barry F."  wrote:
> 
>> 
>>  The reason the code works this way is that normally 
>> -ksp_error_if_not_converged is propagated into the inner (and innerer) 
>> solves and normally it is desirable that these inner solves do not error 
>> simply because they reach the maximum number of iterations since for nested 
>> iterative methods generally we don't need or care if the inner solves 
>> "converge". 
> 
> I fully agree with you on the last part of the above sentence. Thus, this 
> makes me question the first part (which I wasn't aware of): why is 
> error_if_not_converged being propagated to inner solves?

  Because the idea is to catch the problem (and the location in the stack trace 
the very first time it occurs rather than delay until that information is lost 
up at the top level.)

  Jed provides a fine example if this. Say my coarse grid solve is CG and as it 
iterates it finds the problem is indefinite. With -ksp_error_if_not_converged I 
want it to stop there immediately. Or if somewhere inside I have an LU solver, 
if it finds a zero pivot I want the code to stop right there and give me the 
full stack trace. Stopping all the way at the top in the KSPSolve() tells me 
nothing useful to debug the problem of a five level nested LU solver failure.

   The diverged its is a special case that is annoying, I agree, I proposed a 
couple of (not great) solutions to that specific problem. But the solution is 
not to toss the baby out with the bath water. The propagation is a very good 
thing we just need to find a good way to allow overriding the diverged its 
situation.

   Barry


> I'm sure there are good usages, but if one cares that ksp_1 (which depends on 
> ksp_2) converges, why should an error be thrown if ksp_2 does not converge as 
> long as ksp_1 does (I guess this goes along your last paragraph)?
> 
> Thanks,
> Pierre
> 
>>   Of course, in your particular use case this backfires. 
>> 
>>   I'm not sure what the best design fix is. It seems anything will 
>> complicate the code making it harder to maintain. 
>> 
>>   Possible changes:
>>  * if the error_if_not_converged is set directly on a KSP then also 
>> cause it to error on maximum iterations, but not if it is propagated into a 
>> KSP. This would require tracking how the error if not converged got set
>>  * make error_if_not_converged an enum with, for example, the value 
>> __always__ causing the error to be generated even on maximum iterations. 
>> This is easier on the code then above 
>> 
>>   A larger more far-reaching change would be to change the current model and 
>> have the "inner" KSP use CONVERGED_ITERATING instead of DIVERGED_ITERATING 
>> when it doesn't matter if they converge or not: 
>> KSPSetReachMaximumIterationsConverged(), then the outer KSP would call this 
>> on the inner KSP at construction time. This is appealing from code clarity 
>> point of view since now the inner solves return a negative (diverged) reason 
>> when that is acceptable and that is confusing, to return a diverged reason 
>> while everything is fine. For the given use case one would need to do 
>> -pc_bddc_coarse_ksp_error_if_not_converged  
>> -pc_bddc_coarse_ksp_reach_maximum_iterations_converged false but this is 
>> cumbersome, who would remember the second option.
>> 
>>  I'm not really excited by any of my proposed solutions.
>> 
>> Thoughts?
>> 
>> 
>> Barry
>> 
>> 
>> 
>> 
>>> On Oct 20, 2019, at 7:55 AM, Pierre Jolivet via petsc-dev 
>>>  wrote:
>>> 
>>> Hello,
>>> I’m trying to get multilevel solvers to error out when coarse levels are 
>>> not converging, but I’m failing…
>>> Could someone either tell me if this is not possible to do so, or help me 
>>> find what the problem in my options is, please?
>>> (in src/ksp/ksp/examples/tutorials)
>>> $ mpirun -n 8 ./ex71 -pde_type Elasticity -cells 7,9,8 -dim 3 
>>> -pc_bddc_monolithic -pc_bddc_use_faces -pc_bddc_coarse_pc_type none 
>>> -pc_bddc_coarse_ksp_type gmres -pc_bddc_coarse_ksp_converged_reason 
>>> -pc_bddc_coarse_ksp_error_if_not_converged -ksp_converged_reason
>>> Linear pc_bddc_coarse_ solve did not converge due to DIVERGED_ITS 
>>> iterations 1 <- I want to error out here
>>> […]
>>> Linear pc_bddc_coarse_ solve did not converge due to DIVERGED_ITS 
>>> iterations 1
>>> Linear solve converged due to CONVERGED_RTOL iterations 31
>>> $ mpirun -np 1 ./ex1 -ksp_type gmres -pc_type gamg -ksp_converged_reason 
>>> -mg_coarse_pc_type lu -mg_coarse_ksp_max_it 5 -mg_coarse_ksp_type gmres 
>>> -ksp_type fgmres -mg_levels_1_ksp_type fgmres 
>>> -mg_coarse_ksp_error_if_not_converged -mg_coarse_ksp_converged_reason
>>>   Linear mg_coarse_ solve did not converge due to DIVERGED_ITS iterations 5 
>>>  <- I want to error out here
>>> […]
>>>   Linear mg_coarse_ solve did not converge due to DIVERGED_ITS iterations 5
>>> Linear solve converged due to CONVERGED_RTOL iterations 3

Re: [petsc-dev] "participants" on gitlab

2019-10-21 Thread Smith, Barry F. via petsc-dev


> On Oct 21, 2019, at 10:27 AM, Zhang, Hong via petsc-dev 
>  wrote:
> 
> How is the list of participants determined when a MR is created on gitlab? It 
> seems to include everybody by default. Is there any way to shorten the list? 
> Ideally only the participants involved in the particular MR should be picked. 
> Note that currently there is a huge gap between the ''Participate'' and ''On 
> mention'' levels in the notification settings. With the former, I get spammed 
> with notifications whenever a new MR is created. With the later, I won’t 
> receive any notification (even someone replied my comments) unless explicitly 
> @ by someone.

  If you make a MR and then on the right side slide over Notifications button 
do you not get all mail on the MR even if you don't do the Participate?  It 
seems for me it is slide over automatically.

  We don't know GitLab any better than you so you could also google to find out 
how to handle this. I now have a bunch of mail filters that help me from 
getting useless crap about MR but still get more perhaps than I want. You may 
need to add mail filters also.


   Barry



> 
> Hong (Mr.)



Re: [petsc-dev] BlockGetIndices and GetBlockIndices

2019-10-21 Thread Smith, Barry F. via petsc-dev

Fix one of presumably many bugs related to the rush to add PetscLayout too IS 
without understanding the ramifications.

https://gitlab.com/petsc/petsc/merge_requests/2191


> On Oct 21, 2019, at 8:34 AM, Pierre Jolivet via petsc-dev 
>  wrote:
> 
> 
>> On 21 Oct 2019, at 3:01 PM, Jed Brown  wrote:
>> 
>> Pierre Jolivet via petsc-dev  writes:
>> 
 On 21 Oct 2019, at 7:52 AM, Smith, Barry F.  wrote:
 
 
 
> On Oct 21, 2019, at 12:23 AM, Pierre Jolivet  
> wrote:
> 
> 
> 
>> On 21 Oct 2019, at 7:11 AM, Smith, Barry F.  wrote:
>> 
>> 
>> 
>>> On Oct 20, 2019, at 11:52 PM, Pierre Jolivet 
>>>  wrote:
>>> 
>>> 
>>> 
 On 21 Oct 2019, at 6:42 AM, Smith, Barry F.  wrote:
 
 Could you provide a use case where you want to access/have a block 
 size of a IS that is not an ISBlock? 
>>> 
>>> In the end, all I really want is get access to the underlying 
>>> is->data->idx without having to worry about the subclass of is.
>>> I don’t have such a use case, but I don’t think this is really related 
>>> to what I want to achieve (or maybe it is…).
>> 
>> ISGetIndices()
> 
> Not true for ISBlock with bs > 1.
 
 Certainly suppose to be, is there a bug?
 
 static PetscErrorCode ISGetIndices_Block(IS in,const PetscInt *idx[])
 {
 IS_Block   *sub = (IS_Block*)in->data;
 PetscErrorCode ierr;
 PetscInt   i,j,k,bs,n,*ii,*jj;
 
 PetscFunctionBegin;
 ierr = PetscLayoutGetBlockSize(in->map, );CHKERRQ(ierr);
 
 Dang, there is that stupid layout stuff again. Who put this crap in. 
 
 ierr = PetscLayoutGetLocalSize(in->map, );CHKERRQ(ierr);
 n   /= bs;
 if (bs == 1) *idx = sub->idx;
 else {
>>> 
>>> There it is, I don’t want this if/else. ISGetBlockIndices would have been a 
>>> function always returning sub->idx.
>> 
>> Your code still can't skip the branch because ISGeneral can have bs>1.
> 
> Ah, OK, got confused by the lack of bs in the ISCreateGeneral constructor.
> 
> Thanks,
> Pierre
> 
>> Block size in IS means "these indices go together" while ISBlock imposes
>> the further constraint: "indices in each block are contiguous".  So you
>> can't just take the code you quoted where it returns the raw idx:
>> 
>> if (!isblock) {
>> ISGetIndices(is,);
>> ISLocalToGlobalMappingCreate(comm,1,n,indices,PETSC_COPY_VALUES,mapping);
>> ISRestoreIndices(is,);
>> } else {
>> ISGetBlockSize(is,);
>> ISBlockGetIndices(is,);
>> ISLocalToGlobalMappingCreate(comm,bs,n/bs,indices,PETSC_COPY_VALUES,mapping);
>> ISBlockRestoreIndices(is,);
>> }
>> 
>> 
>> PetscErrorCode  ISGetBlockSize(IS is,PetscInt *size)
>> {
>> PetscErrorCode ierr;
>> 
>> PetscFunctionBegin;
>> ierr = PetscLayoutGetBlockSize(is->map, size);CHKERRQ(ierr);
>> PetscFunctionReturn(0);
>> }
>> 
> 



Re: [petsc-dev] "participants" on gitlab

2019-10-21 Thread Jed Brown via petsc-dev
All "developers" are listed as able to grant (optional) approvals --
approval from codeowners/integrators is still needed regardless of those
optional approvals.  We should perhaps remove that because I don't know
a way to have some able to approve without the notification problem you
mention below.  Unfortunately, I think that reduces incentive to review,
and we're always stressed for reviewing resources.

"Zhang, Hong via petsc-dev"  writes:

> How is the list of participants determined when a MR is created on gitlab? It 
> seems to include everybody by default. Is there any way to shorten the list? 
> Ideally only the participants involved in the particular MR should be picked. 
> Note that currently there is a huge gap between the ''Participate'' and ''On 
> mention'' levels in the notification settings. With the former, I get spammed 
> with notifications whenever a new MR is created. With the later, I won’t 
> receive any notification (even someone replied my comments) unless explicitly 
> @ by someone.
>
> Hong (Mr.)


[petsc-dev] "participants" on gitlab

2019-10-21 Thread Zhang, Hong via petsc-dev
How is the list of participants determined when a MR is created on gitlab? It 
seems to include everybody by default. Is there any way to shorten the list? 
Ideally only the participants involved in the particular MR should be picked. 
Note that currently there is a huge gap between the ''Participate'' and ''On 
mention'' levels in the notification settings. With the former, I get spammed 
with notifications whenever a new MR is created. With the later, I won’t 
receive any notification (even someone replied my comments) unless explicitly @ 
by someone.

Hong (Mr.)

Re: [petsc-dev] BlockGetIndices and GetBlockIndices

2019-10-21 Thread Pierre Jolivet via petsc-dev


> On 21 Oct 2019, at 3:01 PM, Jed Brown  wrote:
> 
> Pierre Jolivet via petsc-dev  writes:
> 
>>> On 21 Oct 2019, at 7:52 AM, Smith, Barry F.  wrote:
>>> 
>>> 
>>> 
 On Oct 21, 2019, at 12:23 AM, Pierre Jolivet  
 wrote:
 
 
 
> On 21 Oct 2019, at 7:11 AM, Smith, Barry F.  wrote:
> 
> 
> 
>> On Oct 20, 2019, at 11:52 PM, Pierre Jolivet 
>>  wrote:
>> 
>> 
>> 
>>> On 21 Oct 2019, at 6:42 AM, Smith, Barry F.  wrote:
>>> 
>>> Could you provide a use case where you want to access/have a block size 
>>> of a IS that is not an ISBlock? 
>> 
>> In the end, all I really want is get access to the underlying 
>> is->data->idx without having to worry about the subclass of is.
>> I don’t have such a use case, but I don’t think this is really related 
>> to what I want to achieve (or maybe it is…).
> 
> ISGetIndices()
 
 Not true for ISBlock with bs > 1.
>>> 
>>> Certainly suppose to be, is there a bug?
>>> 
>>> static PetscErrorCode ISGetIndices_Block(IS in,const PetscInt *idx[])
>>> {
>>> IS_Block   *sub = (IS_Block*)in->data;
>>> PetscErrorCode ierr;
>>> PetscInt   i,j,k,bs,n,*ii,*jj;
>>> 
>>> PetscFunctionBegin;
>>> ierr = PetscLayoutGetBlockSize(in->map, );CHKERRQ(ierr);
>>> 
>>> Dang, there is that stupid layout stuff again. Who put this crap in. 
>>> 
>>> ierr = PetscLayoutGetLocalSize(in->map, );CHKERRQ(ierr);
>>> n   /= bs;
>>> if (bs == 1) *idx = sub->idx;
>>> else {
>> 
>> There it is, I don’t want this if/else. ISGetBlockIndices would have been a 
>> function always returning sub->idx.
> 
> Your code still can't skip the branch because ISGeneral can have bs>1.

Ah, OK, got confused by the lack of bs in the ISCreateGeneral constructor.

Thanks,
Pierre

> Block size in IS means "these indices go together" while ISBlock imposes
> the further constraint: "indices in each block are contiguous".  So you
> can't just take the code you quoted where it returns the raw idx:
> 
> if (!isblock) {
>  ISGetIndices(is,);
>  ISLocalToGlobalMappingCreate(comm,1,n,indices,PETSC_COPY_VALUES,mapping);
>  ISRestoreIndices(is,);
> } else {
>  ISGetBlockSize(is,);
>  ISBlockGetIndices(is,);
>  ISLocalToGlobalMappingCreate(comm,bs,n/bs,indices,PETSC_COPY_VALUES,mapping);
>  ISBlockRestoreIndices(is,);
> }
> 
> 
> PetscErrorCode  ISGetBlockSize(IS is,PetscInt *size)
> {
>  PetscErrorCode ierr;
> 
>  PetscFunctionBegin;
>  ierr = PetscLayoutGetBlockSize(is->map, size);CHKERRQ(ierr);
>  PetscFunctionReturn(0);
> }
> 



Re: [petsc-dev] ksp_error_if_not_converged in multilevel solvers

2019-10-21 Thread Pierre Jolivet via petsc-dev


> On 21 Oct 2019, at 3:04 PM, Jed Brown  wrote:
> 
> Pierre Jolivet via petsc-dev  writes:
> 
>> On Oct 20, 2019, at 6:07 PM, "Smith, Barry F."  wrote:
>> 
>>> 
>>>  The reason the code works this way is that normally 
>>> -ksp_error_if_not_converged is propagated into the inner (and innerer) 
>>> solves and normally it is desirable that these inner solves do not error 
>>> simply because they reach the maximum number of iterations since for nested 
>>> iterative methods generally we don't need or care if the inner solves 
>>> "converge". 
>> 
>> I fully agree with you on the last part of the above sentence. Thus, this 
>> makes me question the first part (which I wasn't aware of): why is 
>> error_if_not_converged being propagated to inner solves?
>> I'm sure there are good usages, but if one cares that ksp_1 (which depends 
>> on ksp_2) converges, why should an error be thrown if ksp_2 does not 
>> converge as long as ksp_1 does (I guess this goes along your last paragraph)?
> 
> What if the user is debugging a singular or indefinite coarse operator
> when they expect it to be SPD?  Sure, they could set that flag
> directly for the coarse KSP via the options database.

I most often end up debugging bottom-up (or coarse to fine) instead of 
top-down, but you probably have some convincing arguments that I should do it 
the other way around.

Re: [petsc-dev] ksp_error_if_not_converged in multilevel solvers

2019-10-21 Thread Jed Brown via petsc-dev
Pierre Jolivet via petsc-dev  writes:

> On Oct 20, 2019, at 6:07 PM, "Smith, Barry F."  wrote:
>
>> 
>>   The reason the code works this way is that normally 
>> -ksp_error_if_not_converged is propagated into the inner (and innerer) 
>> solves and normally it is desirable that these inner solves do not error 
>> simply because they reach the maximum number of iterations since for nested 
>> iterative methods generally we don't need or care if the inner solves 
>> "converge". 
>
> I fully agree with you on the last part of the above sentence. Thus, this 
> makes me question the first part (which I wasn't aware of): why is 
> error_if_not_converged being propagated to inner solves?
> I'm sure there are good usages, but if one cares that ksp_1 (which depends on 
> ksp_2) converges, why should an error be thrown if ksp_2 does not converge as 
> long as ksp_1 does (I guess this goes along your last paragraph)?

What if the user is debugging a singular or indefinite coarse operator
when they expect it to be SPD?  Sure, they could set that flag
directly for the coarse KSP via the options database.


Re: [petsc-dev] BlockGetIndices and GetBlockIndices

2019-10-21 Thread Jed Brown via petsc-dev
Pierre Jolivet via petsc-dev  writes:

>> On 21 Oct 2019, at 7:52 AM, Smith, Barry F.  wrote:
>> 
>> 
>> 
>>> On Oct 21, 2019, at 12:23 AM, Pierre Jolivet  
>>> wrote:
>>> 
>>> 
>>> 
 On 21 Oct 2019, at 7:11 AM, Smith, Barry F.  wrote:
 
 
 
> On Oct 20, 2019, at 11:52 PM, Pierre Jolivet  
> wrote:
> 
> 
> 
>> On 21 Oct 2019, at 6:42 AM, Smith, Barry F.  wrote:
>> 
>> Could you provide a use case where you want to access/have a block size 
>> of a IS that is not an ISBlock? 
> 
> In the end, all I really want is get access to the underlying 
> is->data->idx without having to worry about the subclass of is.
> I don’t have such a use case, but I don’t think this is really related to 
> what I want to achieve (or maybe it is…).
 
 ISGetIndices()
>>> 
>>> Not true for ISBlock with bs > 1.
>> 
>>  Certainly suppose to be, is there a bug?
>> 
>> static PetscErrorCode ISGetIndices_Block(IS in,const PetscInt *idx[])
>> {
>>  IS_Block   *sub = (IS_Block*)in->data;
>>  PetscErrorCode ierr;
>>  PetscInt   i,j,k,bs,n,*ii,*jj;
>> 
>>  PetscFunctionBegin;
>>  ierr = PetscLayoutGetBlockSize(in->map, );CHKERRQ(ierr);
>> 
>> Dang, there is that stupid layout stuff again. Who put this crap in. 
>> 
>>  ierr = PetscLayoutGetLocalSize(in->map, );CHKERRQ(ierr);
>>  n   /= bs;
>>  if (bs == 1) *idx = sub->idx;
>>  else {
>
> There it is, I don’t want this if/else. ISGetBlockIndices would have been a 
> function always returning sub->idx.

Your code still can't skip the branch because ISGeneral can have bs>1.
Block size in IS means "these indices go together" while ISBlock imposes
the further constraint: "indices in each block are contiguous".  So you
can't just take the code you quoted where it returns the raw idx:

if (!isblock) {
  ISGetIndices(is,);
  ISLocalToGlobalMappingCreate(comm,1,n,indices,PETSC_COPY_VALUES,mapping);
  ISRestoreIndices(is,);
} else {
  ISGetBlockSize(is,);
  ISBlockGetIndices(is,);
  ISLocalToGlobalMappingCreate(comm,bs,n/bs,indices,PETSC_COPY_VALUES,mapping);
  ISBlockRestoreIndices(is,);
}


PetscErrorCode  ISGetBlockSize(IS is,PetscInt *size)
{
  PetscErrorCode ierr;

  PetscFunctionBegin;
  ierr = PetscLayoutGetBlockSize(is->map, size);CHKERRQ(ierr);
  PetscFunctionReturn(0);
}



Re: [petsc-dev] BlockGetIndices and GetBlockIndices

2019-10-21 Thread Pierre Jolivet via petsc-dev


> On 21 Oct 2019, at 7:52 AM, Smith, Barry F.  wrote:
> 
> 
> 
>> On Oct 21, 2019, at 12:23 AM, Pierre Jolivet  
>> wrote:
>> 
>> 
>> 
>>> On 21 Oct 2019, at 7:11 AM, Smith, Barry F.  wrote:
>>> 
>>> 
>>> 
 On Oct 20, 2019, at 11:52 PM, Pierre Jolivet  
 wrote:
 
 
 
> On 21 Oct 2019, at 6:42 AM, Smith, Barry F.  wrote:
> 
> Could you provide a use case where you want to access/have a block size 
> of a IS that is not an ISBlock? 
 
 In the end, all I really want is get access to the underlying 
 is->data->idx without having to worry about the subclass of is.
 I don’t have such a use case, but I don’t think this is really related to 
 what I want to achieve (or maybe it is…).
>>> 
>>> ISGetIndices()
>> 
>> Not true for ISBlock with bs > 1.
> 
>  Certainly suppose to be, is there a bug?
> 
> static PetscErrorCode ISGetIndices_Block(IS in,const PetscInt *idx[])
> {
>  IS_Block   *sub = (IS_Block*)in->data;
>  PetscErrorCode ierr;
>  PetscInt   i,j,k,bs,n,*ii,*jj;
> 
>  PetscFunctionBegin;
>  ierr = PetscLayoutGetBlockSize(in->map, );CHKERRQ(ierr);
> 
> Dang, there is that stupid layout stuff again. Who put this crap in. 
> 
>  ierr = PetscLayoutGetLocalSize(in->map, );CHKERRQ(ierr);
>  n   /= bs;
>  if (bs == 1) *idx = sub->idx;
>  else {

There it is, I don’t want this if/else. ISGetBlockIndices would have been a 
function always returning sub->idx.

Thanks,
Pierre

>ierr = PetscMalloc1(bs*n,);CHKERRQ(ierr);
>*idx = jj;
>k= 0;
>ii   = sub->idx;
>for (i=0; i  for (j=0; jjj[k++] = bs*ii[i] + j;
>  }
>  PetscFunctionReturn(0);
> }
> 
> 
>> 
 Again, my goal is just to avoid having to write something like: 
 https://www.mcs.anl.gov/petsc/petsc-dev/src/vec/is/utils/isltog.c.html#line322
 If things were done as in ISLocalToGlobalMapping (which I know is not the 
 same at all as IS/ISBlock), I think this could just read:
 ISGetBlockIndices(is,)
 ISLocalToGlobalMappingCreate(comm,bs,n/bs,indices,PETSC_COPY_VALUES,mapping)
>>> 
>>> If you want your code to take advantage of the underlying structure of the 
>>> IS (be it strided, blocked) for optimizations then you HAVE to go through 
>>> the subclasses by design and nature (for example VecScatterCreate() takes 
>>> advantage of the structure of the IS as does 
>>> ISLocalToGlobalMappingCreate()). There is no universal concept of a block 
>>> size in IS (despite the crap that other people put in using PetscLayout 
>>> which has nothing to do with ISBlock and is completely buggy.) nor should 
>>> there be or is there a need for. 
>> 
>> OK then.
>> 
>> Thanks,
>> Pierre
>> 
>>> Barry
>>> 
>>> 
>>> 
>>> 
 
 Thanks,
 Pierre
 
> 
>> On Oct 16, 2019, at 2:50 AM, Pierre Jolivet via petsc-dev 
>>  wrote:
>> 
>> Hello,
>> I’m trying to understand what is the rationale for naming a function 
>> ISBlockGetIndices and another ISLocalToGlobalMappingGetBlockIndices 
>> (BlockGet vs. GetBlock).
>> Also, it looks to me like the implementation of ISBlockGetIndices is 
>> somehow less versatile than ISLocalToGlobalMappingGetBlockIndices.
>> Indeed, I may call ISLocalToGlobalMappingGetBlockIndices with an 
>> underlying non-block IS, while I can’t for ISBlockGetIndices, so a check 
>> must be performed by the user, e.g., 
>> https://www.mcs.anl.gov/petsc/petsc-current/src/vec/is/is/examples/tutorials/ex3.c.html#line58
>>  or this (IMHO) ugly code duplication 
>> https://www.mcs.anl.gov/petsc/petsc-dev/src/vec/is/utils/isltog.c.html#line322.
>> 
>> Thoughts and/or comments? Would it make sense to add an 
>> ISGetBlockIndices/ISRestoreBlockIndices or would that be too confusing 
>> for the user?
>> 
>> Thanks,
>> Pierre