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

add gc.collect() to the memleak test from #31313 #31671

Closed
dimpase opened this issue Apr 15, 2021 · 14 comments
Closed

add gc.collect() to the memleak test from #31313 #31671

dimpase opened this issue Apr 15, 2021 · 14 comments

Comments

@dimpase
Copy link
Member

dimpase commented Apr 15, 2021

the memleak test added in #31313 does not use gc.collect(), which leads to random errors sometimes.

CC: @koffie @jhpalmieri @vbraun

Component: graph theory

Author: Dima Pasechnik

Branch: 872e9ad

Reviewer: John Palmieri

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

@dimpase dimpase added this to the sage-9.3 milestone Apr 15, 2021
@dimpase
Copy link
Member Author

dimpase commented Apr 15, 2021

Author: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Apr 15, 2021

@dimpase
Copy link
Member Author

dimpase commented Apr 15, 2021

New commits:

872e9adgc.collect() to normalise get_memory_usage() output

@dimpase
Copy link
Member Author

dimpase commented Apr 15, 2021

Commit: 872e9ad

@dimpase
Copy link
Member Author

dimpase commented Apr 15, 2021

comment:3

Please review!

@jhpalmieri
Copy link
Member

comment:4

I still get the same occasional doctest failure:

sage [t/31671/graphs/add_gccollect_to_memleak_test] % ./sage -t src/sage/graphs/bipartite_graph.py
Running doctests with ID 2021-04-15-09-29-14-4df85c26.
Git branch: t/31671/graphs/add_gccollect_to_memleak_test
Using --optional=build,dochtml,homebrew,pip,sage,sage_spkg
Doctesting 1 file.
sage -t --warn-long 103.4 --random-seed=0 src/sage/graphs/bipartite_graph.py
**********************************************************************
File "src/sage/graphs/bipartite_graph.py", line 329, in sage.graphs.bipartite_graph.BipartiteGraph.__init__
Failed example:
    print(round(get_memory_usage() - start_mem))
Expected:
    0.0
Got:
    1.0
**********************************************************************
1 item had failures:
   1 of  14 in sage.graphs.bipartite_graph.BipartiteGraph.__init__
    [320 tests, 1 failure, 1.32 s]
----------------------------------------------------------------------
sage -t --warn-long 103.4 --random-seed=0 src/sage/graphs/bipartite_graph.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 1.4 seconds
    cpu time: 1.3 seconds
    cumulative wall time: 1.3 seconds

@dimpase
Copy link
Member Author

dimpase commented Apr 15, 2021

comment:5

well, then there is a real platform-specific (macOS) bug.
Still, this patch makes sense to get in now and here.

@dimpase
Copy link
Member Author

dimpase commented Apr 15, 2021

comment:6

or should we tag it as a # known bug ?

@jhpalmieri
Copy link
Member

comment:7

I agree that the patch here can't hurt, and it could certainly help in some cases. We can merge this and separately can try to track down what's going on with OS X.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@vbraun
Copy link
Member

vbraun commented Apr 17, 2021

Changed branch from u/dimpase/graphs/add_gccollect_to_memleak_test to 872e9ad

@slel
Copy link
Member

slel commented Jun 28, 2021

Changed commit from 872e9ad to none

@slel
Copy link
Member

slel commented Jun 28, 2021

comment:9

Discussed again:

@dimpase
Copy link
Member Author

dimpase commented Jun 28, 2021

comment:10

well, there is a bug in bipartite_graph, no way around it...

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