Skip to content

CVode.poolshrink(1) deletes unused mechanism pools.#2033

Merged
nrnhines merged 4 commits into
masterfrom
hines/poolshrink
Oct 28, 2022
Merged

CVode.poolshrink(1) deletes unused mechanism pools.#2033
nrnhines merged 4 commits into
masterfrom
hines/poolshrink

Conversation

@nrnhines

@nrnhines nrnhines commented Oct 21, 2022

Copy link
Copy Markdown
Member

To delete unused mechanism memory pools (e.g. when you have cleared the model) use

cv = h.CVode()
cv.poolshrink(1) # deletes pools if number of items used is 0
cv.poolshrink() # prints current number of items used and number of items allocated for double arrays and Datum arrays.

Needs documentation.

@nrnhines nrnhines requested a review from iomaganaris October 21, 2022 10:51
@azure-pipelines

azure-pipelines Bot commented Oct 21, 2022

Copy link
Copy Markdown

✔️ d2256f2 -> Azure artifacts URL

@alexsavulescu

alexsavulescu commented Oct 21, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Just thinking out loud, would it be possible to automatically call poolshrink and avoid leaving this in the user's hands?

@azure-pipelines

azure-pipelines Bot commented Oct 21, 2022

Copy link
Copy Markdown

✔️ c8d7eb6 -> Azure artifacts URL

iomaganaris
iomaganaris reviewed Oct 24, 2022
View reviewed changes

@iomaganaris iomaganaris left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I did some tests and this seems to work as expected.
I am just thinking that this should be added to the documentation and maybe we could also add a test about this?

@nrnhines

nrnhines commented Oct 24, 2022

Copy link
Copy Markdown
Member Author

possible to automatically call poolshrink

I suppose that if a model is empty (no pool item in use for any pool) that all the memory could be reclaimed. But poolshrink
started out as a temporary measure to solve a specific issue and I think will be superseded by the std::vector reorganization.

@alexsavulescu

alexsavulescu commented Oct 25, 2022

Copy link
Copy Markdown
Member

will be superseded by the std::vector reorganization

I believe that will still imply calls to clear() and/or shrink_to_fit()

@alexsavulescu

alexsavulescu commented Oct 27, 2022

Copy link
Copy Markdown
Member

@iomaganaris maybe we can merge this and tackle docs/handling in another PR?

iomaganaris
iomaganaris reviewed Oct 27, 2022
View reviewed changes

@iomaganaris iomaganaris left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure we can merge this without the docs at the moment. I just have one question looking at the code.

Comment thread src/nrniv/cxprop.cpp Outdated
@codecov-commenter

codecov-commenter commented Oct 28, 2022

Copy link
Copy Markdown

Codecov Report

Merging #2033 (5fbf643) into master (1346a84) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #2033      +/-   ##
==========================================
- Coverage   48.64%   48.63%   -0.02%     
==========================================
  Files         519      519              
  Lines      116039   116068      +29     
==========================================
+ Hits        56447    56449       +2     
- Misses      59592    59619      +27
Impacted Files Coverage Δ
src/nrncvode/cvodeobj.cpp 85.66% <0.00%> (-0.69%) ⬇️
src/nrniv/cxprop.cpp 57.45% <0.00%> (-3.65%) ⬇️
src/sundials/cvodes/cvodes.c 32.74% <0.00%> (+0.07%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@azure-pipelines

azure-pipelines Bot commented Oct 28, 2022

Copy link
Copy Markdown

✔️ 5fbf643 -> Azure artifacts URL

@nrnhines nrnhines merged commit d4254f0 into master Oct 28, 2022
@nrnhines nrnhines deleted the hines/poolshrink branch October 28, 2022 07:55
@iomaganaris iomaganaris mentioned this pull request Nov 9, 2022
Merged
iomaganaris pushed a commit that referenced this pull request Nov 9, 2022
@nrnhines @alexsavulescu @iomaganaris
CVode.poolshrink(1) deletes unused mechanism pools. (#2033)
2e52ffc 
* CVode.poolshrink(1) deletes unused mechanism pools.

Co-authored-by: Alexandru Săvulescu <alexandru.savulescu@epfl.ch>
nrnhines added a commit that referenced this pull request Nov 9, 2022
@iomaganaris @alexsavulescu
@olupton @nrnhines
Cherry-pick multiple commits to make release/8.2 functional (#2074)
eb19ae0 
* refactoring: drop java code (#1937)

* Update removed_features.rst

* Fine tune clang-format and fix CI (#2041)

* Delete return statement to avoid ASan error.

* CVode.poolshrink(1) deletes unused mechanism pools. (#2033)

* Pin bokeh<3, don't pin matplotlib (#2048)

* Add list(...) for new matplotlib.

Co-authored-by: Alexandru Săvulescu <alexandru.savulescu@epfl.ch>
Co-authored-by: Olli Lupton <oliver.lupton@epfl.ch>
Co-authored-by: nrnhines <michael.hines@yale.edu>
alexsavulescu added a commit that referenced this pull request Dec 15, 2022
@iomaganaris @alexsavulescu
@olupton @nrnhines
Cherry-pick multiple commits to make release/8.2 functional (#2074)
a921c4e 
* refactoring: drop java code (#1937)

* Update removed_features.rst

* Fine tune clang-format and fix CI (#2041)

* Delete return statement to avoid ASan error.

* CVode.poolshrink(1) deletes unused mechanism pools. (#2033)

* Pin bokeh<3, don't pin matplotlib (#2048)

* Add list(...) for new matplotlib.

Co-authored-by: Alexandru Săvulescu <alexandru.savulescu@epfl.ch>
Co-authored-by: Olli Lupton <oliver.lupton@epfl.ch>
Co-authored-by: nrnhines <michael.hines@yale.edu>
pramodk pushed a commit to BlueBrain/neurodamus that referenced this pull request Jun 1, 2023
@iomaganaris
Added poolshrink command when clearing the model from neuronsimulator…
3870214 
…/nrn#2033
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iomaganaris iomaganaris iomaganaris left review comments
@olupton olupton olupton approved these changes
@alexsavulescu alexsavulescu alexsavulescu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nrnhines @alexsavulescu @codecov-commenter @olupton @iomaganaris