-
Notifications
You must be signed in to change notification settings - Fork 47
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
Added a lazy result object. #1421
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1421 +/- ##
===========================================
- Coverage 83.36% 83.24% -0.13%
===========================================
Files 161 161
Lines 13247 13396 +149
===========================================
+ Hits 11043 11151 +108
- Misses 2204 2245 +41 ☔ View full report in Codecov by Sentry. |
Not sure what you mean.
I'd say so. Something to be discussed: Should changes to the result just affect the OptimizerResult instance or should they be written to file?
Why aren't they loaded on demand then? |
Sorry meant caching. If we load very big hessians, we might want to throw them out later again? 🤔
That's an interesting thought. I would say per default no, as it would not be a result object created by optimization but changed thereafter. But a function would probably be good to propagate changes to the file?
Not entirely sure, it might be because in the OptimizerResult they are attributes while here they are properties? And so since LazyResult is a subclass, it will get initiated with those attributes as well 🤔 When calling |
I have been using hdfdict [1] with pydantic objects, no issues so far in my basic use cases (including numpy arrays), which has lazy loading on by default. Hopefully the lazy loading persists when a pydantic data object is reconstructed from HDF5 via hdfdict, but I haven't tested this yet. So one alternative to this PR would be to refactor our results to be pydantic classes. I guess this might anyway occur when PEtab Result is supported in pyPESTO. [1] https://github.com/SiggiGue/hdfdict/blob/master/hdfdict/hdfdict.py |
Co-authored-by: Daniel Weindl <[email protected]>
@dilpath as we talked about, I think it makes sense to merge it in and when |
Yes, fine for me 👍 |
Added a result object, that reads the results only if needed. Connected to #1405
Things to consider still
None