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/60a1948e59517edd5702a5e277d78229b7...

Toby pointed it out and fixed it in this commit, which went into 'next':

https://bitbucket.org/petsc/petsc/commits/38e7336fef8d310d693744ab39306ec098...

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/9837ea968140067d7f638ef40a3e6ee2b9...

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/9837ea968140067d7f638ef40a3e6ee2b9... 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.