improve boost_graph.pyx

sagemath / sage

Main repository of SageMath

https://www.sagemath.org

Other

1.33k stars 453 forks source link

improve boost_graph.pyx #26554

Closed dcoudert closed 5 years ago

dcoudert commented 5 years ago

Improve the boost graph interface to avoid using .vertices().

After that, one sorted operation involving vertex labels comparisons remains, in method min_spanning_tree.

CC: @tscrim @fchapoton @jhpalmieri

Component: graph theory

Author: David Coudert

Branch/Commit: 71fa004

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/26554

dcoudert commented 5 years ago

comment:1

This one took me some time as error messages of Cython are sometimes hard to understand...

New commits:

4cb7735 trac #26554: improve boost_graph.pyx

dcoudert commented 5 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
 Improve the boost graph interface to avoid using `.vertices()`.

-After that, 1 `sorted` operation involving vertex labels comparisons remains, in method `min_spanning_tree`.
+After that, *one* `sorted` operation involving vertex labels comparisons remains, in method `min_spanning_tree`.

dcoudert commented 5 years ago

Commit: 4cb7735

dcoudert commented 5 years ago

Branch: public/26554_boost_graph_pyx

tscrim commented 5 years ago

comment:2

The patchbot reports two failures that are likely caused from this patch (I cannot check right now):

sage -t --long src/sage/homology/simplicial_complex.py  # 2 doctests failed
sage -t --long src/sage/matroids/utilities.py  # 1 doctest failed

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 4cb7735 to 71fa004

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

71fa004 trac #26554: correct failing doctests in simplicial_complex and matroids

dcoudert commented 5 years ago

comment:4

I have corrected the doctests, but it would be good if someone could check that the results are effectively correct. I have no doubt in src/sage/matroids/utilities.py as the output of a test taking as input the changed matrix is unchanged. I have more difficulty with simplicial_complex...

tscrim commented 5 years ago

comment:5

The changes to simplicial_complex.py are trivially equivalent (same presentation of groups except for the generator names). I am very mildly worried that the output may not always be consistent, but since we are in such early beta stages, I think getting this in and testing it will be the best way.

John, I am cc-ing you to also note this ticket in case we do start seeing random failures in simplicial_complex.py.

tscrim commented 5 years ago

Reviewer: Travis Scrimshaw

dcoudert commented 5 years ago

comment:6

Thank you !

jhpalmieri commented 5 years ago

comment:7

Thanks for the heads up.

vbraun commented 5 years ago

Changed branch from public/26554_boost_graph_pyx to 71fa004