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

FIX: Fix excluded model contributions #113

Merged
merged 13 commits into from Dec 9, 2019
Merged

Conversation

bocklund
Copy link
Member

@bocklund bocklund commented Dec 9, 2019

This PR factors computing data quantities out into its own function (get_data_quantities) (out of fit_formation_energy) and fixes several bugs, which were all based on the units of excluded_model_contributions. The units of model parameters are [qty]/mole-formula, while the datasets and Model quantities are call [qty]/mole-atoms. Tests for these cases are in tests/test_parameter_generation_utils.py.

  • excluded_model_contributions were not being applied to endmembers, a holdover from when idmix was hardcoded as the excluded model contribution. Tested by test_get_data_quantities_magnetic_energy.
  • excluded_model_contributions had incorrectly been multiplied by an extra _site_ratio_normalization. Tested by test_get_data_quantities_mixing_entropy and test_get_data_quantities_AL_NI_VA_interaction
  • Since the code is now in a function, it was also pulled out of the inner loop over candidate models, since the data are not dependent on the model representation.

This PR also includes a small fix not overriding the SER reference state data from SGTE when creating new databases in parameter selection

@bocklund bocklund changed the title [FIX]: Fix excluded model contributions FIX: Fix excluded model contributions Dec 9, 2019
@bocklund bocklund merged commit 5c1e8c7 into master Dec 9, 2019
@bocklund bocklund deleted the paramselect-data-qty-refactor branch April 20, 2021 03:44
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

1 participant