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

populate "Generated by" in EnergyAdjustment generated by MP2020 and fix serialization bug #1960

Merged
merged 3 commits into from Sep 29, 2020

Conversation

rkingsbury
Copy link
Contributor

@rkingsbury rkingsbury commented Sep 21, 2020

Populate the cls kwarg when creating EnergyAdjustment within MaterialsProject2020Compatibility. This information will be helpful in tracking the provenance of energy adjustments.

Now that the cls kwarg is populated as intended, executing ComputedEntry.energy_adjustments on an entry that has been processed with the MP2020 correction scheme should generate output similar to

[CompositionEnergyAdjustment:
   Name: MP2020 anion correction (oxide)
   Value: -5.920 eV
   Uncertainty: 0.014 eV
   Description: Composition-based energy adjustment (-0.740 eV/atom x 8.0 atoms)
   Generated by: MaterialsProject2020Compatibility,
 CompositionEnergyAdjustment:
   Name: MP2020 GGA/GGA+U mixing correction (Fe)
   Value: -8.728 eV
   Uncertainty: 0.036 eV
   Description: Composition-based energy adjustment (-2.182 eV/atom x 4.0 atoms)
   Generated by: MaterialsProject2020Compatibility]

Whereas before, the "Generated by:" line read None

In addition, I discovered that several classes inherited from EnergyAdjustment (including CompositionEnergyAdjustment, which is used by MP2020) were not serializing properly. This was because the description attribute was being modified during init and because some kwargs were not stored as attributes. This PR also fixes those problems.

@rkingsbury rkingsbury changed the title Mp2020 adj populate "Generated by" in EnergyAdjustment generated by MP2020 Sep 21, 2020
@rkingsbury rkingsbury changed the title populate "Generated by" in EnergyAdjustment generated by MP2020 [WIP] populate "Generated by" in EnergyAdjustment generated by MP2020 and fix serialization bug Sep 24, 2020
@rkingsbury rkingsbury changed the title [WIP] populate "Generated by" in EnergyAdjustment generated by MP2020 and fix serialization bug populate "Generated by" in EnergyAdjustment generated by MP2020 and fix serialization bug Sep 24, 2020
@mkhorton
Copy link
Member

Unless you want to communicate that people should be able to change adj_per_atom after instatiation, you probably want to set it with self._adj_per_atom (similar to eg self._value previously), likewise with uncertainty. Otherwise this looks good.

@shyuep shyuep merged commit 2016a28 into materialsproject:master Sep 29, 2020
@rkingsbury rkingsbury deleted the mp2020-adj branch September 29, 2020 17:49
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.

None yet

3 participants