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 <[email protected]> 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. > >
signature.asc
Description: Digital signature
