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

various details in combinat #31918

Closed
fchapoton opened this issue Jun 6, 2021 · 13 comments
Closed

various details in combinat #31918

fchapoton opened this issue Jun 6, 2021 · 13 comments

Comments

@fchapoton
Copy link
Contributor

  • about range(0, *)
  • about isinstance() or isinstance()

CC: @tscrim @slel @trevorkarn

Component: combinatorics

Author: Frédéric Chapoton

Branch: 2967d9f

Reviewer: Trevor K. Karn

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

@fchapoton fchapoton added this to the sage-9.4 milestone Jun 6, 2021
@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/31918

@fchapoton
Copy link
Contributor Author

New commits:

ee487bevarious details about combinat (range and isinstance)

@fchapoton
Copy link
Contributor Author

Commit: ee487be

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 6, 2021

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

2967d9fone detail

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 6, 2021

Changed commit from ee487be to 2967d9f

@fchapoton
Copy link
Contributor Author

comment:3

bot is morally green, please review

@trevorkarn
Copy link
Contributor

Reviewer: Trevor K Karn

@trevorkarn
Copy link
Contributor

comment:4

Looks good to me.

@trevorkarn
Copy link
Contributor

Changed reviewer from Trevor K Karn to Trevor K. Karn

@vbraun
Copy link
Member

vbraun commented Jun 29, 2021

Changed branch from u/chapoton/31918 to 2967d9f

@darijgr
Copy link
Contributor

darijgr commented Jul 26, 2021

Changed commit from 2967d9f to none

@darijgr
Copy link
Contributor

darijgr commented Jul 26, 2021

comment:7
-        result = [None] * len(grouping)
+        result = []
         j = 0
-        for i in range(len(grouping)):
-            result[i] = sum(self[j:j+grouping[i]])
-            j += grouping[i]
+        for gi in grouping:
+            result.append(sum(self[j:j + gi]))
+            j += gi

Sure this is an improvement?

sage: def xfun():
....:     a = []
....:     for i in range(10000):
....:         a.append(i)
....:     return a
....:
sage: def yfun():
....:     a = [None]*10000
....:     for i in range(10000):
....:         a[i] = i
....:     return a
....:
sage:
sage: timeit.eval("xfun()")
625 loops, best of 3: 1.6 ms per loop
sage: timeit.eval("yfun()")
625 loops, best of 3: 1.32 ms per loop

(I get even starker differences on occasion.)
Appending to a list over and over might be worse than initializing it to the right length.

@fchapoton
Copy link
Contributor Author

comment:8

indeed. Undone among the other changes in #32310.

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