Opened 6 years ago

Closed 6 years ago

#13699 closed defect (fixed)

Another bug in graph products....

Reported by: ncohen Owned by: jason, ncohen, rlm
Priority: major Milestone: sage-5.5
Component: graph theory Keywords:
Cc: dcoudert Merged in: sage-5.5.rc0
Authors: Nathann Cohen, David Coudert Reviewers: David Coudert
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by ncohen)

The bug was found by Geoff Tims. It has been introduced in #12791, exactly as already spotted in #13091.

After loops and multiple edges, I begin to hate directed graphs now.

I *HATE* to see Sage return wrong answers. HATE it.

Nathann

APPLY:

Attachments (2)

trac_13699.patch​ (4.6 KB) - added by ncohen 6 years ago.
trac_13699-edge_iterators.patch​ (2.8 KB) - added by ncohen 6 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by ncohen

I re-read the other products changed by #12791. They look fine.

God, I HATE it.

Changed 6 years ago by ncohen

comment:2 Changed 6 years ago by ncohen

  • Status changed from new to needs_review

comment:3 follow-up: Changed 6 years ago by dcoudert

  • Reviewers set to David Coudert
  • Status changed from needs_review to needs_work

Sorry about that. It means that the doctests were not sufficient to identify the error.

The patch is working, passes all tests, and the documentation is correctly build. However, could you add an extra doctest to check that strong product is the union of tensor and cartesian products. This is working correctly with this patch but not without this patch.

Something else: I'm pretty sure that this part ​http://trac.sagemath.org/sage_trac/attachment/ticket/12791/trac_12791-edge-iterator.patch of patch #12791 has not been included in Sage since most of the graph product functions are without iterators. Don't know what to do.

comment:4 in reply to: ↑ 3 Changed 6 years ago by ncohen

  • Status changed from needs_work to needs_review

Patch updated.

Something else: I'm pretty sure that this part ​http://trac.sagemath.org/sage_trac/attachment/ticket/12791/trac_12791-edge-iterator.patch of patch #12791 has not been included in Sage since most of the graph product functions are without iterators. Don't know what to do.

Yep, it hasn't. Well, the only thing to do is to ask Jeroen I guess.

Nathann

comment:5 Changed 6 years ago by ncohen

  • Authors changed from Nathann Cohen to Nathann Cohen, David Coudert
  • Description modified (diff)

Changed 6 years ago by ncohen

comment:6 Changed 6 years ago by ncohen

The third patch from #12791, which hasnot been merged into Sage, could not be applied anymore after this patch. I just rebased it on top of this ticket and added after trac_13699.patch.

Nathann

comment:7 Changed 6 years ago by ncohen

Well, no need to ask Jeroen anymore. I had begun to send him an email, but as the file needed to be rebased anyway...:-)

Nathann

comment:8 Changed 6 years ago by dcoudert

  • Status changed from needs_review to positive_review

Thanks Nathann. It's now working properly.

comment:9 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.5.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.