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

remove getstate and setstate from Stack, and base argument from init #201

Merged
merged 5 commits into from
Oct 16, 2024

Conversation

bmaranville
Copy link
Member

@bmaranville bmaranville commented Oct 1, 2024

There was a TODO in the Stack class, indicating that the custom __getstate__ and __setstate__ methods might not be needed.

These methods were making it difficult to subclass Stack without implementing even more specialized versions of the methods.

After removing them, deepcopy and serialize/deserialize seem to work fine.
Here is a test script to run and check. It shows that

  • the thickness Calculation function is properly linked in the Stack.thickness.slot after deepcopy and serialize/deserialize
  • the thickness attribute Parameter remains properly linked in Expression objects after deepcopy and serialize/deserialize
from refl1d.names import Slab, Stack, SLD
from copy import deepcopy
from bumps.serialize import serialize, deserialize
import pickle
import dill

sample = Slab(SLD(rho=10), thickness=10) | Slab(SLD(rho=10), thickness=20) | Slab(SLD(rho=10), thickness=30)
thickness_plus = sample.thickness + 100 # expression

ser_t, ser_s = deserialize(serialize([thickness_plus, sample]))
assert(ser_t.value == 160)
ser_s[0].thickness.value += 40
assert(ser_t.value == 200)

dc_t, dc_s = deepcopy([thickness_plus, sample])
assert(dc_t.value == 160)
dc_s[0].thickness.value += 40
assert(dc_t.value == 200)

pickle_t, pickle_s = pickle.loads(pickle.dumps([thickness_plus, sample]))
assert (pickle_t.value == 160)
pickle_s[0].thickness.value += 40
assert (pickle_t.value == 200)

dill_t, dill_s = dill.loads(dill.dumps([thickness_plus, sample]))
assert (dill_t.value == 160)
dill_s[0].thickness.value += 40
assert (dill_t.value == 200)

assert(thickness_plus.value == 160)
sample[0].thickness.value += 40
assert(thickness_plus.value == 200)

Copy link
Contributor

@hoogerheide hoogerheide left a comment

Choose a reason for hiding this comment

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

Looks good. Serialization/deserialization on a deepcopy of a regular model is functional.

@bmaranville bmaranville merged commit c6a0ac1 into master Oct 16, 2024
10 checks passed
@bmaranville bmaranville deleted the stack-remove-setstate branch October 16, 2024 18:56
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