Opened 2 years ago
Closed 2 years ago
#26621 closed enhancement (fixed)
clean bliss.pyx
Reported by:  dcoudert  Owned by:  

Priority:  major  Milestone:  sage8.5 
Component:  graph theory  Keywords:  py3, graph 
Cc:  tscrim, dimpase  Merged in:  
Authors:  David Coudert  Reviewers:  Travis Scrimshaw, Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  92c3509 (Commits, GitHub, GitLab)  Commit:  92c3509f81a65bd705ece438714215c845552b62 
Dependencies:  Stopgaps: 
Description
PEP8 and avoid using .edges()
and .vertices()
Change History (10)
comment:1 Changed 2 years ago by
 Branch set to public/26621_bliss
 Cc tscrim dima added
 Commit set to 9e0691ee5c7897f8d4ceca97e23c6bb106ed9992
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Cc dimpase added; dima removed
Dima, see comment:1.
David, Microoptimization, but since you know cdef list done = [False] * n
will be a length n
array of bint
's, you are better to directly allocate such an array (Cython generates better C code and you do not have the overhead of list
).
comment:3 Changed 2 years ago by
 Status changed from needs_review to needs_work
I will do that. I also did a mistake in initializations. I will fix that.
comment:4 Changed 2 years ago by
 Commit changed from 9e0691ee5c7897f8d4ceca97e23c6bb106ed9992 to 1a59d4f412a3a0c2919a4c6f0846bcafb10c679a
Branch pushed to git repo; I updated commit sha1. New commits:
1a59d4f  trac #26621: improvements

comment:6 Changed 2 years ago by
It seems to be a good idea to declare logLnr
as
cdef int logLnr = 0
(in two places in src/sage/graphs/bliss.pyx
)
comment:7 Changed 2 years ago by
 Commit changed from 1a59d4f412a3a0c2919a4c6f0846bcafb10c679a to 92c3509f81a65bd705ece438714215c845552b62
Branch pushed to git repo; I updated commit sha1. New commits:
92c3509  trac #26621: initialize logLnr to 0

comment:8 Changed 2 years ago by
It's certainly better like that. Thanks.
comment:9 Changed 2 years ago by
 Reviewers set to Travis Scrimshaw, Dima Pasechnik
 Status changed from needs_review to positive_review
Thank you Dima. Everything else LGTM.
comment:10 Changed 2 years ago by
 Branch changed from public/26621_bliss to 92c3509f81a65bd705ece438714215c845552b62
 Resolution set to fixed
 Status changed from positive_review to closed
@dima: could you check that variable
logLnr
is always assigned a value in methodsbliss_graph_from_labelled_edges
andbliss_digraph_from_labelled_edges
. I seems that it might be used uninitialized...New commits:
trac #26621: clean bliss.pyx