Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve boost_graph.pyx #26554

Closed
dcoudert opened this issue Oct 25, 2018 · 13 comments
Closed

improve boost_graph.pyx #26554

dcoudert opened this issue Oct 25, 2018 · 13 comments

Comments

@dcoudert
Copy link
Contributor

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 dcoudert added this to the sage-8.5 milestone Oct 25, 2018
@dcoudert
Copy link
Contributor Author

comment:1

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


New commits:

4cb7735trac #26554: improve boost_graph.pyx

@dcoudert

This comment has been minimized.

@dcoudert
Copy link
Contributor Author

Commit: 4cb7735

@dcoudert
Copy link
Contributor Author

Branch: public/26554_boost_graph_pyx

@tscrim
Copy link
Collaborator

tscrim commented Oct 26, 2018

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 26, 2018

Changed commit from 4cb7735 to 71fa004

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 26, 2018

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

71fa004trac #26554: correct failing doctests in simplicial_complex and matroids

@dcoudert
Copy link
Contributor Author

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
Copy link
Collaborator

tscrim commented Oct 26, 2018

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
Copy link
Collaborator

tscrim commented Oct 26, 2018

Reviewer: Travis Scrimshaw

@dcoudert
Copy link
Contributor Author

comment:6

Thank you !

@jhpalmieri
Copy link
Member

comment:7

Thanks for the heads up.

@vbraun
Copy link
Member

vbraun commented Oct 28, 2018

Changed branch from public/26554_boost_graph_pyx to 71fa004

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants