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

Add pickling support to C++ solver dummy #30

Merged
merged 8 commits into from
Apr 17, 2023
Merged

Conversation

IshaanDesai
Copy link
Member

This PR adds functionality to allow for pickling and unpickling of the C++ micro simulation code in the C++ solver dummy. This is based on the pickling support documentation of pybind11.

@IshaanDesai IshaanDesai self-assigned this Apr 14, 2023
@IshaanDesai IshaanDesai added this to the v1.0.0 milestone Apr 14, 2023
@IshaanDesai IshaanDesai marked this pull request as ready for review April 14, 2023 15:08
@erikscheurer
Copy link
Contributor

Some questions from my side, otherwise it looks good.
I think it would be very useful for this PR to be also merged into #29, because if we have a get and set state method, implementing the __deepcopy__ function would be as easy as saying, create a new instance with self._sim_id, get_state from self and set_state of the new instance. This could then be part of the c++ dummy and used without modification by the user or could even be ignored as written in the pybind deepcopy documentation.
If we write in the documentation that the simulation state should be fully contained in the set_state method, it is more clear that not every variable of the object has to be copied as I did in the deepcopy implementation with the checkpoint as well, but only the necessary ones.

@erikscheurer
Copy link
Contributor

I get an invalid state error running:

import pickle
import micro_dummy
sim = micro_dummy.MicroSimulation(2)
pickle.loads(pickle.dumps(sim))

I think this is because py::make_tuple is called on a std::tuple returning a tuple of a tuple ie length 1.
Moving the make_tuple call to the get_state function seemed to solve the issue for me, but I don't know if this is the intended definition of the getState function.

@IshaanDesai IshaanDesai merged commit ffa869f into develop Apr 17, 2023
@IshaanDesai IshaanDesai deleted the pickleable-cpp-dummy branch April 17, 2023 08:15
@IshaanDesai IshaanDesai mentioned this pull request Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants