Closed timmysilv closed 3 months ago
codecov[bot]
commented
3 months ago Attention: Patch coverage is 94.24460% with 8 lines in your changes missing coverage. Please review.
Project coverage is 89.21%. Comparing base (
2df4c6d) to head (ca0398b). Report is 1 commits behind head on develop.
| Files | Patch % | Lines |
|---|---|---|
| mrmustard/lab_dev/circuit_components.py | 84.37% | 5 Missing :warning: |
| mrmustard/physics/representations.py | 82.35% | 3 Missing :warning: |
apchytr
commented
3 months ago Might have missed it but does the serialization keep track of a version? Could be useful even if we arent concerned about backward compatibility
timmysilv
commented
3 months ago it doesn't. can definitely add it, though I'm not sure what it'll add. I'd be curious to know more on how people intend on using this before adding more features. I kept it pretty barebones because I honestly don't know what we want from this tool, and kinda need more info before adding more advanced features to the module. that said, I can add version when saving and pop it when loading if we want that.
apchytr
commented
3 months ago it doesn't. can definitely add it, though I'm not sure what it'll add. I'd be curious to know more on how people intend on using this before adding more features. I kept it pretty barebones because I honestly don't know what we want from this tool, and kinda need more info before adding more advanced features to the module. that said, I can add version when saving and pop it when loading if we want that.
I agree we should keep it barebones for now but I do think theres some benefit including it. At the very least it would make it clear what version the saved files are associated with if breaking changes happen. That is to say only if we don't rely on the static 0.7.3 version. I think either it could be the latest commit hash from develop (temporary until we have actual releases) or we could start updating the pyproject.toml with an alpha (e.g. 0.7.3a1). What do you think?
timmysilv
commented
3 months ago unless you wish for me to turn back time π¦
>>> Dgate([0], x=1.1).serialize()
PosixPath('/home/ubuntu/src/github.com/XanaduAI/MrMustard/.serialize_cache/Dgate_fe40cc9a65e940ec83850ade04a5cac4.json')>>> Circuit([Dgate([0], x=1.1)]).serialize()
PosixPath('/home/ubuntu/src/github.com/XanaduAI/MrMustard/.serialize_cache/Circuit_82b2f9e1484d42ed9feb0d9cca6299a6.json')since the last review, I've made two notable changes:
settings, so you can configure it easily (no more global hehe)remove_after kwarg for load is now False by default@apchytr I've added basic versioning just using __version__. I'm open to adding more informative versioning in the future but I find the alpha suffixes cumbersome to maintain, so I'd rather not do it unless we see clear value. Perhaps this can be motivation to make things more stable? So you may only break things between versions (even the most minor version change)
@zeyueN I've posted examples of all the things above π
timmysilv
commented
3 months ago ok since that last comment things have really changed. Check out the updated PR description for details, but basically only Circuit.serialize() writes to file now.
timmysilv
commented
3 months ago @zeyueN I just added the path property to the circuit serialization. Note that it's only evaluated if the user called make_path, so it'll be empty otherwise. I don't save the _graph property, because it depends on non-serialized/non-functional data (a random number tied to the wires object that isn't serialized either)
Context: For execution in certain environments, it helps to be able to de/serialize objects
Description of the Change:
utils.serializemodule for saving and loading MrMustard objects that implement the interfaceCircuitComponent,BargmannandFock(andNumberto avoid a tensorflow bug)Circuit, consuming info organized by those helpers then writing to 2 (zipped) filesOperationconstructor is now correctParameterSet.all_parametersno longer adds the variables to the constantsImplementation details
CircuitComponentone returns two items, serializable kwargs and non-serializable datanameis present in the component type__init__method, it serializes using values needed for_from_attributes. This is never the case for concrete classes, but a backup if any class does provide itmodesormodes_in + modes_outare present. If neither are, it will fail. Then it passes all params from theParameterSet, adding<p>_trainableand<p>_boundsif it's a variable as opposed to a constantsavewithoutarrays, it creates a.jsonfile. If you providearrays(whichCircuit.serializedoes), it creates a.zipfile with two files in it: the regular JSON file, and a.npzfile that contains the associated numpy data. This helps with portability. If you'd rather not zip things (eg. perf over portability), you can passdo_zip=FalsetosaveBenefits: Useful for certain execution environments
Possible Drawbacks:
ParameterSetcontents are enough to serialize. This was not true for the maps, so I updated their parameter setsmodes_inandmodes_outimply thatin_ket == out_ket, and thatin_bra == out_bramodeswill have all in/out/bra/ket modes equal or emptyRelated GitHub Issues: N/A