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

Added deepcpy functionality and small test for roadrunner. #1347

Merged
merged 8 commits into from
Apr 5, 2024
7 changes: 0 additions & 7 deletions pypesto/objective/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,6 @@ def __init__(
self.pre_post_processor = PrePostProcessor()
self.history = NoHistory()

def __deepcopy__(self, memodict=None) -> "ObjectiveBase":
"""Create deepcopy of objective object."""
other = type(self)() # maintain type for derived classes
for attr, val in self.__dict__.items():
other.__dict__[attr] = copy.deepcopy(val)
return other

# The following has_ properties can be used to find out what values
# the objective supports.
@property
Expand Down
18 changes: 18 additions & 0 deletions pypesto/objective/roadrunner/road_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

Currently does not support sensitivities.
"""
import copy
from collections import OrderedDict
from collections.abc import Sequence
from typing import Optional, Union
Expand Down Expand Up @@ -70,6 +71,23 @@ def __init__(
self.solver_options = solver_options
super().__init__(x_names=x_names)

def __deepcopy__(
self, memodict: Optional[dict] = None
) -> "RoadRunnerObjective":
"""Deepcopy function for RoadRunner objective."""
import roadrunner

other = self.__class__.__new__(self.__class__)

for key in set(self.__dict__.keys()) - {"roadrunner_instance"}:
other.__dict__[key] = copy.deepcopy(self.__dict__[key])
other_rr = roadrunner.RoadRunner()
state = self.roadrunner_instance.saveStateS()
other_rr.loadStateS(state=state)
other.roadrunner_instance = other_rr
Copy link
Member

@dweindl dweindl Apr 2, 2024

Choose a reason for hiding this comment

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

Maybe you can check with the RR devs if __deepcopy__ could be provided by RR.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Actually, if we remove

def __deepcopy__(self, memodict=None) -> "ObjectiveBase":
"""Create deepcopy of objective object."""
other = type(self)() # maintain type for derived classes
for attr, val in self.__dict__.items():
other.__dict__[attr] = copy.deepcopy(val)
return other

this __deepcopy__ function here wouldn't be needed.

Does anybody recall the purpose of ObjectiveBase.__deepcopy__?

Copy link
Member

Choose a reason for hiding this comment

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

There's an option that deepcopys the objective at Problem instantiation.

if copy_objective:
objective = copy.deepcopy(objective)

But I'm not sure why __deepcopy__ is defined, I would expect a default deepcopy(...) to do what is defined in ObjectiveBase.__deepcopy__ already.

Copy link
Member

Choose a reason for hiding this comment

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

I would expect a default deepcopy(...) to do what is defined in ObjectiveBase.__deepcopy__ already.

Exactly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agreed. Will remove the default deep copy and see whether all tests still work, then merge for now. Might be reverted when RR developers add a deepcopy function.

Copy link
Member

Choose a reason for hiding this comment

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

Will remove the default deep copy and see whether all tests still work, then merge for now.

Not only ObjectiveBase.__deepcopy__, but also RoadRunnerObjective.__deepcopy__.

Might be reverted when RR developers add a deepcopy function.

No, that wouldn't require any change then.

Copy link
Collaborator Author

@PaulJonasJost PaulJonasJost Apr 5, 2024

Choose a reason for hiding this comment

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

ah so deepcopy in its original behaviour would use the __set/getstate__ functionality from RR?

Copy link
Member

Choose a reason for hiding this comment

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

deepcopy will fall back to __getstate__/ __setstate__, which exist (see Dilan's comment above)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good 👍🏼 will remove it but keep the tests, as I think we should still make sure it is working?


return other

def get_config(self) -> dict:
"""Return basic information of the objective configuration."""
info = super().get_config()
Expand Down
34 changes: 33 additions & 1 deletion test/base/test_roadrunner.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
"""Test the roadrunner interface."""

import copy
import logging
import os

import benchmark_models_petab as models
import petab
import petabtests
import pytest
Expand Down Expand Up @@ -104,3 +106,33 @@ def _execute_case_rr(case, model_type, version):
)

logger.info(f"Case {version}/{model_type}/{case} passed.")


def test_deepcopy():
"Test that deepcopy works as intended"
PaulJonasJost marked this conversation as resolved.
Show resolved Hide resolved
model_name = "Boehm_JProteomeRes2014"
petab_problem = petab.Problem.from_yaml(
os.path.join(models.MODELS_DIR, model_name, model_name + ".yaml")
)
petab_problem.model_name = model_name
importer = objective_rr.PetabImporterRR(petab_problem)
problem_parameters = petab_problem.x_nominal_free_scaled

problem = importer.create_problem()
obj = problem.objective

problem_copied = copy.deepcopy(problem)
copied_objective = problem_copied.objective

assert obj(problem_parameters) == copied_objective(problem_parameters)

# !!not adviced, only done here for testing purposes!!
obj.roadrunner_instance.removeParameter(
"pSTAT5A_rel", forceRegenerate=False
)
obj.roadrunner_instance.addParameter("pSTAT5A_rel", 0.0, False)
obj.roadrunner_instance.addAssignmentRule(
"pSTAT5A_rel", "(100 * pApB + 200 * pApA * specC17)"
)

assert obj(problem_parameters) != copied_objective(problem_parameters)
Loading