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

Unpickling old objects may not work with new methods and object attributes #183

Open
maij opened this issue Apr 15, 2020 · 1 comment
Open

Comments

@maij
Copy link
Collaborator

maij commented Apr 15, 2020

I encountered an example of this when unpickling an old pulse sequence.

The pulse sequence contains FrequencyRampPulses which don't contain a recently added new attribute phase_reference, which is required for methods such as get_voltage. I'm performing a hotfix for this instance in #204, but this a broader issue which might reoccur.

I had a brief look into a proper fix, but I got a bit lost.
When pickle loads objects it calls __new__ but not __init__, so default values aren't created. I'm beginning to think each class should have default values (not just what's written in __init__) which can be modified with __new__.

@nulinspiratie Do you understand what the issue is here, have you thought about this before?


This is how I think it should approximately work (Foo is good, Bar is bad), although this doesn't really work when all attributes are Parameters that are created during __init__. Could we move parameter creation to __new__?

class Foo():
  var = 42
  def __init__(var=None):
    if var is not None:
        self.var = var

class Bar():
  def __init__(var=None):
    if var is None:
      self.var = 42
    else:
      self.var = var
@nulinspiratie
Copy link
Owner

Hey @maij I have indeed noticed this behaviour before. however, I'm a bit wary of also introducing __new__, mostly because we would have two very similar functions and it would be confusing what part of the initialization goes where. I can understand the Parameter instantiation would go to __new__, but what about functions that link parameters to the config, etc.?
I'm also not sure if adding new would work well with parameter methods (which use @parameter).

I had also come across other issues with pickling pulse sequencing, such as it being fairly slow (dill is even slower than pickle), and that it sometimes causes issues when updating silq / qcodes.

I thought that with the new ParameterNode structure, it should actually be fairly straightforward to serialize a pulse sequence to json and also to create a pulse sequence from json. It should also be a much faster process, and probably also much cleaner. There's also the advantage that the file can be opened by a text editor, so one could also view the pulse sequence without using python.
What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants