[WIP] Make MigrationHop MSONable again

[mkhorton]

Please add test @jmmshn thank you!

[jmmshn]

So it looks like we are hitting a custom as_dict + from_dict

https://github.com/materialsproject/pymatgen/blob/dffcac644662d9657d17f872b3c397c5e5a17df7/pymatgen/symmetry/structure.py#L135

So when the decoding tries to process the symm_structure it does not know what to do.
Can we replace those upstream in pymatgen?

[mkhorton]

From looking at it, it's not even clear to me that that custom as_dict() is necessary at all.

[mkhorton]

That said, why is this an issue upstream?

[jmmshn]

OK so sit down for this one...

So right now I think when MigrationHop.from_dict getting called, and the parser hits the symm_structure it goes into the structure field of SymmetrizedStructure which then sees the
'@class': 'SymmetrizedStructure' part of the dictionary and tries to run the SymmetrizedStructure.as_dict on the structure field of serialized dictionary.

This is all because this line
https://github.com/materialsproject/pymatgen/blob/dffcac644662d9657d17f872b3c397c5e5a17df7/pymatgen/symmetry/structure.py#L140
is not doing what the author thinks it's doing. It is not calling the as_dict of the parent Structure and returning a Structure. It's calling Structure.as_dict on SymmetrizedStructure.

We gotta have some kind of auto test generation for these things...

[mkhorton]

Still confused based on this description, why would it try to run as_dict on a dictionary?

…

On Fri, Oct 15, 2021 at 17:30, JSX ***@***.***> wrote: OK so sit down for this one... So right now I think when MigrationHop.from_dict getting called, and the parser hits the symm_structure it goes into the structure field of SymmetrizedStructure which then sees the ***@***.***': 'SymmetrizedStructure' part of the dictionary and tries to run the SymmetrizedStructure.as_dict on the structure field of serialized dictionary. [image: image] <https://user-images.githubusercontent.com/14003693/137566601-096c2a08-5465-437a-ae48-65a3057d2e53.png> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#264 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAWWWRBIU2YWLMMAQOYTSJDUHDBSJANCNFSM5GAQ5Z6Q> .

[jmmshn]

So the problem right now is

# Hop is some MigrationHop object
d = mg.as_dict()
MigrationHop.from_dict(d)  # errors out
MontyDecoder().process_decoded(d['symm_structure']) # errors out

The decoding errors out even though it's just a dictionary because when it gets to d["symm_structure"]["structure"] it sees a '@class': 'SymmetrizedStructure and just freaks out. (it will try to call the from_dict of SymmetrizedStructure but the present level of the data is actually made from the as_dict of Structure).

That's because this line is absolutely not OK.
https://github.com/materialsproject/pymatgen/blob/dffcac644662d9657d17f872b3c397c5e5a17df7/pymatgen/symmetry/structure.py#L140

[shyuep]

This should be fixed in the pymatgen SymmetrizedStructure side, not in the diffusion package.

[acrutt]

@jmmshn @mkhorton is there an update on if the necessary fixes for this were implemented in pymatgen? I'm now running into issues because this was never resolved.

[mkhorton]

If someone else would like to take over this PR, please do, I think it only needs a test to be able to merge; if I recall I opened this during a debugging session, but I'm not involved in pymatgen-analysis-diffusion so would prefer someone else who is actively using it takes it on.

[acrutt]

I can take over writing a test for this but was confused by the follow-up discussion. Is there still an issue with encoding SymmetrizedStructure as a dict?

[acrutt]

This issues has been resolved in a new PR that was successfully merged: #312

[mkhorton]

Fantastic, thanks @acrutt!

[jmmshn]

Thanks, @acrutt I'm on leave for the rest of this month so have been slow with emails.

[jmmshn]

So I think the underlying bug was still not fixed in pymatgen.
It's being addressed in a PR now after that all the code should work as expected.

[jmmshn]

@acrutt if you tried to serialize->deserialize with host_symm_structure does it throws an error still?

[acrutt]

I'm not familiar with that part of the issue. When I create a MigrationHop with the host_symm_struct attribute initialized, the .as_dict() method works for me. I'm using pymatgen version 2022.7.19.

[jmmshn]

So the bug only triggers a problem if you read from a db or read via loadfn due to the order in which things load.

If you see a '@class': 'SymmetrizedStructure' after you run hop.as_dict() then the program will error out when you dumpfn -> loadfn any object that contains the hop.