-
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
Saves pypesto and python version to the problem. #1382
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1382 +/- ##
===========================================
- Coverage 83.96% 81.66% -2.30%
===========================================
Files 160 160
Lines 13051 13055 +4
===========================================
- Hits 10958 10662 -296
- Misses 2093 2393 +300 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to have that information somewhere.
Not sure whether it should be part of Problem
or not. Is the assumption that one result file contains on results obtained from the same pypesto/python version? Then it would be sufficient to have it once per file. Otherwise, it should be saved with each Problem/OptimizerResult. The latter is probably safer, and makes it easier to read the version number to account for potential changes in the result structure.
Okay for me as a start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it's useful! are there cases where it's not possible to save the problem to hdf5?
Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com>
No there should not be any, but you could theoretically specify NOT to save it.
Yes correct that is the assumption currently. I think long term having it in the OptimizerResult would perhaps be better with a set of other "metadata" collections. My list of things I would like to somewhat safe would be the following (happy for any additional ones)
I think for many things it might be useful to alter the |
Could change class TaskResult(dict):
def __init__(self):
self.set_common_metadata()
def set_common_metadata(self):
self.metadata = {
"python_version": ...,
"pypesto_version": ...,
} and then e.g. class OptimizerResult(TaskResult):
def __init__(self, ...):
super().__init__()
def set_task_metadata(self, optimizer: pypesto.optimize.Optimizer, engine: ...):
self.metadata['optimizer'] = type(optimizer)
self.metadata['engine'] = type(engine)
... Then just need to make sure the |
As the problem is generally saved alongside the result, it gives access to the version of pypesto and python that was used for optimization.
Would something like "metadata" of a result object make sense? Basically informative string representations of the engine, optimizer, problem/objective, alongside necessary versions?