ICB-DCM / pyPESTO

python Parameter EStimation TOolbox
https://pypesto.readthedocs.io
BSD 3-Clause "New" or "Revised" License
206 stars 44 forks source link

Saves pypesto and python version to the problem. #1382

Closed PaulJonasJost closed 1 month ago

PaulJonasJost commented 2 months ago

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?

codecov-commenter commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.66%. Comparing base (fefefd5) to head (77341e3).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

PaulJonasJost commented 2 months ago

I agree that it's useful! are there cases where it's not possible to save the problem to hdf5?

No there should not be any, but you could theoretically specify NOT to save it.

Is the assumption that one result file contains on results obtained from the same pypesto/python version?

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 __str__ representation and just hand that over?

dilpath commented 2 months ago

Could change class OptimizerResult(dict), class ProfileResult(dict), etc. to instead inherit from some class TaskResult(dict) that looks like

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 TaskResult.metadata dict is saved in each result storage method, and call OptimizerResult.set_task_metadata(...) inside pypesto.optimize.minimize after creating the OptimizerResult.