Opened 7 years ago
Closed 7 years ago
#15493 closed defect (fixed)
qqbar.ANRoot creates tons of copies of interval fields
Reported by: | mmezzarobba | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.3 |
Component: | number fields | Keywords: | memory, qqbar |
Cc: | Merged in: | ||
Authors: | Marc Mezzarobba | Reviewers: | Vincent Delecroix, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | b8b4265 (Commits) | Commit: | b8b426514bd28afe51ab863bf7d5cea1a77a9f91 |
Dependencies: | Stopgaps: |
Description (last modified by )
sage.rings.qqbar.ANRoot._interval_fast()
creates new interval fields by type(self._interval.parent())(prec)
, thus bypassing the cache provided by RealIntervalField()
. Computations with algebraic numbers sometimes need gigabytes of memory to store these parents and the associated coercion structures.
Before (6.2.beta5):
sage: set_random_seed(42) sage: m = get_memory_usage() sage: %time l = list(QQbar.random_element(poly_degree=3) == 0 for i in range(4000)) CPU times: user 10.7 s, sys: 24 ms, total: 10.7 s Wall time: 10.7 s sage: get_memory_usage(m) 7.77734375
With patch:
sage: set_random_seed(42) sage: m = get_memory_usage() sage: %time l = list(QQbar.random_element(poly_degree=3) == 0 for i in range(4000)) CPU times: user 10.5 s, sys: 40 ms, total: 10.5 s Wall time: 10.5 s sage: get_memory_usage(m) 0.34375
Change History (20)
comment:1 Changed 7 years ago by
- Branch set to u/mmezzarobba/ticket/15493-memleak
- Commit set to 17eece2087c144371dcf68ff7ebf2b74c690dba9
- Status changed from new to needs_review
comment:2 follow-up: ↓ 4 Changed 7 years ago by
- Status changed from needs_review to needs_work
Please use the correct doctest syntax:
EXAMPLES:: sage: 1 + 1 2 More text:: sage: 2 + 2 4
comment:3 Changed 7 years ago by
- Commit changed from 17eece2087c144371dcf68ff7ebf2b74c690dba9 to 26079857fa777d2f7962a964dd95a667917800b2
Branch pushed to git repo; I updated commit sha1. New commits:
2607985 | Doctest syntax |
comment:4 in reply to: ↑ 2 Changed 7 years ago by
- Status changed from needs_work to needs_review
comment:5 Changed 7 years ago by
- Milestone changed from sage-6.0 to sage-6.1
comment:6 Changed 7 years ago by
comment:7 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:8 Changed 7 years ago by
- Keywords memory qqbar added
- Summary changed from Memory leak in QQbar to qqbar.ANRoot creates tons of copies of interval fields
comment:9 follow-up: ↓ 10 Changed 7 years ago by
I just tried your example with version 0.1 of ore_algebra (from the RISC homepage), but got an error:
sage: rec.generalized_series_solutions(1, infolevel=5) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) <ipython-input-4-ebd2bbf4a9be> in <module>() ----> 1 rec.generalized_series_solutions(Integer(1), infolevel=Integer(5)) TypeError: generalized_series_solutions() got an unexpected keyword argument 'infolevel'
Maybe you did use a more recent version of ore_algebra?
Paul
comment:10 in reply to: ↑ 9 Changed 7 years ago by
Replying to zimmerma:
Maybe you did use a more recent version of ore_algebra?
It could be that I used the git version, yes. Does it work without infolevel
?
comment:11 follow-up: ↓ 12 Changed 7 years ago by
Does it work without infolevel?
yes and no (with Sage 6.1 and ore_algebra 0.1):
sage: rec.generalized_series_solutions(1) ... ValueError: factorization of 0 not defined
Paul
comment:12 in reply to: ↑ 11 Changed 7 years ago by
- Description modified (diff)
Replying to zimmerma:
Does it work without infolevel?
yes and no (with Sage 6.1 and ore_algebra 0.1):
You are right, my example did not work with ore_algebra 0.1 (I used a development version that does not seem to be publicly available yet).
comment:13 Changed 7 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:14 follow-up: ↓ 17 Changed 7 years ago by
Hi Marc,
To mention a trac ticket, use :trac:15493
instead of #15493. That way, there is a nice hyperlink in the documentation!
Once that done, it is ready for positive review.
Vincent
comment:15 Changed 7 years ago by
- Status changed from needs_review to needs_work
comment:16 Changed 7 years ago by
- Commit changed from 26079857fa777d2f7962a964dd95a667917800b2 to b8b426514bd28afe51ab863bf7d5cea1a77a9f91
Branch pushed to git repo; I updated commit sha1. New commits:
b8b4265 | #ticket → :trac:`ticket`
|
comment:17 in reply to: ↑ 14 Changed 7 years ago by
Replying to vdelecroix:
To mention a trac ticket, use :trac:
15493
instead of #15493. That way, there is a nice hyperlink in the documentation!
Yes, I keep forgetting! Thanks!
comment:18 Changed 7 years ago by
- Status changed from needs_work to needs_review
comment:19 Changed 7 years ago by
- Reviewers set to Vincent Delecroix, Jeroen Demeyer
- Status changed from needs_review to positive_review
Hello,
Wonderful. Thanks.
Vincent
comment:20 Changed 7 years ago by
- Branch changed from u/mmezzarobba/ticket/15493-memleak to b8b426514bd28afe51ab863bf7d5cea1a77a9f91
- Resolution set to fixed
- Status changed from positive_review to closed
New commits: