[petsc-dev] SF broken in master !

[Tobin Isaac]

On Wed, Oct 15, 2014 at 12:52:24AM -0500, Tobin Isaac wrote:
> On Wed, Oct 15, 2014 at 12:04:47AM -0500, Jed Brown wrote:
> > Barry Smith <bsmith at mcs.anl.gov> writes:
> > 
> > >   and yet it is not broke in next. I thought that was impossible!
> > 
> > This commit was bad:
> > 
> >   https://bitbucket.org/petsc/petsc/commits/60a1948e59517edd5702a5e277d78229b7015096
> > 
> > Toby pointed it out and fixed it in this commit, which went into 'next':
> > 
> >   https://bitbucket.org/petsc/petsc/commits/38e7336fef8d310d693744ab39306ec09879f8c2
> > 
> > Meanwhile, Matt didn't put this in his branch and went on to make other
> > changes to SF, including a similar commit:
> > 
> >   https://bitbucket.org/petsc/petsc/commits/9837ea968140067d7f638ef40a3e6ee2b94657e5
> > 
> > Again, Toby pointed out the bug in this commit, which Matt had to
> > resolve when merging to 'next' because it conflicted with Toby's correct
> > version.  The tests certainly did not pass in Matt's branch, but they
> > passed in 'next' because Toby's branch fixed the bug.
> > 
> > Ultimately, Matt merged to 'master' without merging Toby's bug fix.
> > 
> > I have now merged Toby's branch and 'master' works again.  Thanks, Toby.
> 
> The praise might be misplaced: their bad commit assumed that sf->nroots
> is always as big as the index space of the leaves; my commit assumed
> that sf->nleaves was always big enough.  For sparse leaves, neither
> may be the case (see Michael's comment on the first commit above).  I
> though Matt had a commit that was the "right" think to do...

This is the one:

https://bitbucket.org/petsc/petsc/commits/9837ea968140067d7f638ef40a3e6ee2b94657e5

I think the actual problem is that Matt fixed a typo in the merge into
next and it didn't get rerere'd into master.

  Toby

> 
>   Toby
> 
> > 
> > 
> > Note that prior to this merge, the following was the only difference
> > between 'master' and 'next':
> > 
> > diff --git c/src/vec/is/sf/interface/sf.c w/src/vec/is/sf/interface/sf.c
> > index af2dc58..26a286a 100644
> > --- c/src/vec/is/sf/interface/sf.c
> > +++ w/src/vec/is/sf/interface/sf.c
> > @@ -794,7 +794,7 @@ PetscErrorCode PetscSFGetMultiSF(PetscSF sf,PetscSF *multi)
> >      ierr = PetscMalloc1(sf->nleaves,&remote);CHKERRQ(ierr);
> >      for (i=0; i<sf->nleaves; i++) {
> >        remote[i].rank  = sf->remote[i].rank;
> > -      remote[i].index = outoffset[sf->mine ? sf->mine[i] : 1];
> > +      remote[i].index = outoffset[sf->mine ? sf->mine[i] : i];
> >      }
> >      ierr = PetscSFDuplicate(sf,PETSCSF_DUPLICATE_RANKS,&sf->multi);CHKERRQ(ierr);
> >      ierr = PetscSFSetGraph(sf->multi,inoffset[sf->nroots],sf->nleaves,NULL,PETSC_COPY_VALUES,remote,PETSC_OWN_POINTER);CHKERRQ(ierr);
> > 
> > 
> > To avoid this in the future, I suggest
> > 
> >   1. Run tests on topic branches, especially those tests related to the
> >   modification.  If you're tinkering with SF, the least you can do is
> >   run the SF tests.
> > 
> >   2. Fix content bugs in the topic branch instead of sneaking it into a
> >   merge commit that's oh so easy to forget.
> 
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20141015/0b5f5dc2/attachment.sig>