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

Fix parallelization by fixing pickling and un-pickling of CADET-dll #26

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

ronald-jaepel
Copy link
Collaborator

@ronald-jaepel ronald-jaepel commented Nov 19, 2024

As described in #25 with CADET-Python v1.0.0 the parallelization is broken because we can not pickle ctype pointers.

My proposed solution is to remove all pointers from the state, only save the _cadet_path and then on __getstate__ re-load the dll and pointers in the child-thread.

ToDo:

  • Fix pickling
  • Add test

@ronald-jaepel ronald-jaepel linked an issue Nov 19, 2024 that may be closed by this pull request
@ronald-jaepel ronald-jaepel force-pushed the fix/pickle_capi branch 3 times, most recently from 8cf9717 to 516635e Compare November 19, 2024 13:36
Copy link
Contributor

@schmoelder schmoelder left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this and for adding tests!

tests/test_parallelization.py Outdated Show resolved Hide resolved
@schmoelder
Copy link
Contributor

Just out of curiosity, can you elaborate what function __getstate__ serves?

…on tests

Fixed pickle support by removing all ctypes pointers from the state in CadetDLLRunner.__getstate__ and recreating the dll interface in CadetDLLRunner.__setstate__ .
Fixed "no attribute __frozen" error by casting Cadet state into addict.Dict on Cadet.__setstate__ .
@ronald-jaepel
Copy link
Collaborator Author

Just out of curiosity, can you elaborate what function __getstate__ serves?

__getstate__ get's a pickleable state of the object. Usually, that's just self.__dict__, but by overriding __getstate__ we can change that. We can, e.g. remove all non-pickleable parameters and then change __setstate__ to re-create the removed parameters.

@ronald-jaepel ronald-jaepel merged commit 9cd4160 into master Nov 19, 2024
3 checks passed
@ronald-jaepel ronald-jaepel deleted the fix/pickle_capi branch November 19, 2024 14:35
@ronald-jaepel ronald-jaepel removed the request for review from schmoelder November 19, 2024 14:35
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.

Fix multi-threading PicklingError
2 participants