Added deepcpy functionality and small test for roadrunner.

[PaulJonasJost]

Deepcopy functionality is needed for profiling.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.49%. Comparing base (b791e7f) to head (219596d).

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

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1347      +/-   ##
===========================================
+ Coverage    84.33%   84.49%   +0.16%     
===========================================
  Files          157      157              
  Lines        12911    12906       -5     
===========================================
+ Hits         10888    10905      +17     
+ Misses        2023     2001      -22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dweindl]

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

[dilpath]

also can use their __setstate__ and __getstate__

https://github.com/sys-bio/roadrunner/blob/92d063b8877d121c2c1f878fcc1b5ee093b7ec0b/wrappers/Python/roadrunner/roadrunner.i#L1386-L1392

[dweindl]

Actually, if we remove

pyPESTO/pypesto/objective/base.py

Lines 59 to 64 in 321ee9d

	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__?

[dilpath]

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

pyPESTO/pypesto/problem/base.py

Lines 107 to 108 in 34e89b3

	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.

[dweindl]

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

Exactly.

[PaulJonasJost]

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.

[dweindl]

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.

[PaulJonasJost]

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

[dweindl]

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

[PaulJonasJost]

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