Skip to content

Reduce the peak memory usage of skillmodels#76

Merged
hmgaudecker merged 23 commits intomainfrom
reduce-mem-usage
Sep 11, 2024
Merged

Reduce the peak memory usage of skillmodels#76
hmgaudecker merged 23 commits intomainfrom
reduce-mem-usage

Conversation

@hmgaudecker
Copy link
Copy Markdown
Member

It looks like memory consumption of loglike_and_grad is rising about linearly in the number of observations (and probably also parameters) at present, making it the limiting factor for real-world applications. This PR aims at reducing peak memory usage.

Table to keep track of mem usage of pixi run mem-on-clean-repo will be in MEMTRACKER.md. Delete upon merging.

the sigma_point algorithm chosen.
sigma_weights (jax.numpy.array): 1d array of length n_sigma with non-negative
sigma weights.
transition_info (dict): Dict with the entries "func" (the actual transition
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.

Was the docstring of transition_info outdated or did you actually change this? If you changed it, why is the mapping from columns to positions no longer needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I did not change any functionality, but did check that the only thing ever used was the function. No idea when this stopped being required, for sure had been the case 2.5 years ago.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

(so yes, I did actually change from passing a dict to passing the function only)

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.

Maybe this was before we went full dags? But if it wasn't used you can of course remove it.

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 92.59259% with 8 lines in your changes missing coverage. Please review.

Project coverage is 91.43%. Comparing base (bbd4ce4) to head (f04041d).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/skillmodels/maximization_inputs.py 90.12% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #76      +/-   ##
==========================================
+ Coverage   91.42%   91.43%   +0.01%     
==========================================
  Files          39       40       +1     
  Lines        3066     3070       +4     
==========================================
+ Hits         2803     2807       +4     
  Misses        263      263              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hmgaudecker
Copy link
Copy Markdown
Member Author

For the record: The 8 lines of missed coverage are _get_jnp_params_vec, which does not have tests. Does not seem to be an issue.

@hmgaudecker hmgaudecker merged commit b7975a6 into main Sep 11, 2024
@hmgaudecker hmgaudecker deleted the reduce-mem-usage branch September 11, 2024 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants