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

101 in memory model loading #102

Merged
merged 11 commits into from
Aug 15, 2024

Conversation

kkovary
Copy link
Contributor

@kkovary kkovary commented Aug 10, 2024

closes #101

Changelogs

This PR adds support for loading models directly into memory without saving them to a local cache. This is particularly useful for environments with limited local storage, such as serverless deployments.

Current implementation:

  • A new class InMemoryModelStore is created, inheriting from ModelStore.
  • The download method is overridden to load model data directly into memory.
  • The load method is modified to work with the in-memory data.

An alternative approach could be to add a flag to ModelStore with something like in_memory that when set to True, use in-memory loading logic; otherwise, use current caching behavior.

  • enumerate the changes of that PR.

Checklist:

  • Was this PR discussed in an issue? It is recommended to first discuss a new feature into a GitHub issue before opening a PR.
  • Add tests to cover the fixed bug(s) or the new introduced feature(s) (if appropriate).
  • Update the API documentation if a new function is added, or an existing one is deleted. Eventually consider making a new tutorial for new features.
  • Write concise and explanatory changelogs below.
  • If possible, assign one of the following labels to the PR: feature, fix or test (or ask a maintainer to do it for you).

discussion related to that PR

@kkovary kkovary requested a review from maclandrol as a code owner August 10, 2024 21:56
Copy link
Member

@maclandrol maclandrol left a comment

Choose a reason for hiding this comment

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

Great feature !

Could you add some test and potentially something in the tutorial to showcase the feature ?

@maclandrol
Copy link
Member

Also ruff is complaining.

@kkovary
Copy link
Contributor Author

kkovary commented Aug 10, 2024

Could you add some test and potentially something in the tutorial to showcase the feature ?

yes definitely!

@kkovary
Copy link
Contributor Author

kkovary commented Aug 10, 2024

Also ruff is complaining.

I made this commit to fix the ruff issue

@maclandrol
Copy link
Member

I will wait for the tutorial to merge.

@kkovary kkovary requested a review from maclandrol August 14, 2024 19:35
@kkovary
Copy link
Contributor Author

kkovary commented Aug 15, 2024

I will wait for the tutorial to merge.

I added a short tutorial section for InMemoryModelStore

Copy link
Member

@maclandrol maclandrol left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @kkovary

@maclandrol maclandrol merged commit 40414ae into datamol-io:main Aug 15, 2024
3 checks passed
@kkovary kkovary deleted the 101-in-memory-model-loading branch August 15, 2024 18:33
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.

Add support for in-memory model loading without local caching
2 participants