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

BoundSymbol constructor to be cached #1576

Merged
merged 4 commits into from
Feb 4, 2021
Merged

BoundSymbol constructor to be cached #1576

merged 4 commits into from
Feb 4, 2021

Conversation

mloubout
Copy link
Contributor

@mloubout mloubout commented Feb 3, 2021

Fixes #1575

Switch from __init_finalize to __new__ to make sure the extra u.function is cought by the cache and added extra line to clear_cache test to test it (breaks on master)

@mloubout mloubout added API api (symbolics, types, ...) bug-py minor-patch labels Feb 3, 2021
@codecov
Copy link

codecov bot commented Feb 3, 2021

Codecov Report

Merging #1576 (185f26e) into master (1e4210e) will decrease coverage by 24.78%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1576       +/-   ##
===========================================
- Coverage   86.26%   61.47%   -24.79%     
===========================================
  Files         217      180       -37     
  Lines       31285    20378    -10907     
  Branches     4155     3519      -636     
===========================================
- Hits        26988    12528    -14460     
- Misses       3807     7247     +3440     
- Partials      490      603      +113     
Impacted Files Coverage Δ
devito/types/basic.py 86.01% <100.00%> (-8.57%) ⬇️
benchmarks/user/tools/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
examples/seismic/elastic/operators.py 0.00% <0.00%> (-100.00%) ⬇️
examples/seismic/elastic/wavesolver.py 0.00% <0.00%> (-100.00%) ⬇️
examples/seismic/test_seismic_utils.py 0.00% <0.00%> (-100.00%) ⬇️
examples/seismic/viscoelastic/operators.py 0.00% <0.00%> (-100.00%) ⬇️
examples/seismic/viscoelastic/wavesolver.py 0.00% <0.00%> (-100.00%) ⬇️
examples/seismic/viscoacoustic/operators.py 0.00% <0.00%> (-98.81%) ⬇️
examples/seismic/tti/wavesolver.py 0.00% <0.00%> (-96.83%) ⬇️
examples/seismic/tti/operators.py 0.00% <0.00%> (-95.84%) ⬇️
... and 151 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e4210e...185f26e. Read the comment docs.

u._C_symbol
# All u, u(inds) and u.function through u._c_symbol added to cache
assert(len(_SymbolCache) == cache_size + 3)
del u
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's this del for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as test below force clear cache because its done at the end of the code block not start

@FabioLuporini
Copy link
Contributor

manually triggered CI-gpu: green

merging

@FabioLuporini FabioLuporini merged commit 6bc87d2 into master Feb 4, 2021
@FabioLuporini FabioLuporini deleted the bound-sym branch February 4, 2021 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API api (symbolics, types, ...) bug-py
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BoundSymbol leads to memory leaks
2 participants