Opened 2 years ago
Closed 2 years ago
#26282 closed enhancement (fixed)
Avoid comparison of vertex labels in MIP (Step 2)
Reported by:  dcoudert  Owned by:  

Priority:  major  Milestone:  sage8.4 
Component:  graph theory  Keywords:  
Cc:  tscrim, chapoton  Merged in:  
Authors:  David Coudert  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  6acb781 (Commits, GitHub, GitLab)  Commit:  6acb781fc22f76e5cf544fe68daefb0c1a3c03c4 
Dependencies:  Stopgaps: 
Description
Avoid comparison of vertex labels in graph.py
.
Change History (11)
comment:1 Changed 2 years ago by
 Branch set to public/26282_avoid_comparison_of_vertex_labels
 Commit set to 8e3e85cff2110f86cecc13f75694d984188c5a08
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Commit changed from 8e3e85cff2110f86cecc13f75694d984188c5a08 to 5221e6aa674537650478cdf06e3a98d2b19ec5a0
Branch pushed to git repo; I updated commit sha1. New commits:
5221e6a  trac #26282: resolve merge conflict with 8.4.beta5

comment:3 Changed 2 years ago by
 Commit changed from 5221e6aa674537650478cdf06e3a98d2b19ec5a0 to 2dfc524918880059b9d1ae08fc5b490d4a6e5c06
Branch pushed to git repo; I updated commit sha1. New commits:
2dfc524  trac #26282: use def instead of lambdas

comment:4 Changed 2 years ago by
I change frozenset
wrappers from lambda to def. Should I try to do the same for remaining lambda statements like this ?
weight = lambda x: x if x in RR else 1
comment:5 Changed 2 years ago by
 Cc tscrim chapoton added
comment:6 Changed 2 years ago by
I think you can get rid of
def B(e): return b[frozenset(e)]
def R(e): return r[frozenset(e)]
def S(e): return frozenset(e)
as they are not simplifying the code (or changing the readability IMO.
I would still use six.iteritems
for better memory usage:
 g_mad = g.subgraph([v for v,l in six.iteritems(p.get_values(d)) if l>m ]) + d_val = p.get_values(d) + g_mad = g.subgraph([v for v,l in d_val.items() if l > m ])
In general, I don't get the point of that change.
More standard convention and faster:
lists = dict((v, []) for v in self) +lists = {v: [] for v in self}
sage: %timeit dict((i,1) for i in range(100)) 100000 loops, best of 3: 16.6 µs per loop sage: %timeit {i:1 for i in range(100)} 100000 loops, best of 3: 11.7 µs per loop
not fuv in L
> fuv not in L
.
comment:7 Changed 2 years ago by
 Commit changed from 2dfc524918880059b9d1ae08fc5b490d4a6e5c06 to 6acb781fc22f76e5cf544fe68daefb0c1a3c03c4
comment:8 Changed 2 years ago by
I have implemented all your comments.
comment:9 Changed 2 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
Thanks. LGTM.
comment:10 Changed 2 years ago by
Thank you !
comment:11 Changed 2 years ago by
 Branch changed from public/26282_avoid_comparison_of_vertex_labels to 6acb781fc22f76e5cf544fe68daefb0c1a3c03c4
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
trac #26282: avoid comparison of vertex labels in MIP