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: Minimizer: CompositionSet objects set for removal are not removed #343

Merged
merged 1 commit into from May 30, 2021

Conversation

bocklund
Copy link
Collaborator

The new_free_stable_compset_indices are created after phase removal here, but these new stable indices with the phases removed are essentially reset here (by the code removed in this change).

This is just one way to address this issue, maybe the most straightforward? An alternative could be set the phase amounts to the phases scheduled for removal to zero and to leave in the code that reconstructs the new_stable_compset_indices (which won't include the phases with new zero phase amounts).

The removed code could also fix a bug where a fixed phase is included in the free stable phase indices if it has non-zero phase amount.

@codecov
Copy link

codecov bot commented May 29, 2021

Codecov Report

Merging #343 (3232734) into develop (657f5ba) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #343   +/-   ##
========================================
  Coverage    89.70%   89.70%           
========================================
  Files           44       44           
  Lines         4255     4255           
========================================
  Hits          3817     3817           
  Misses         438      438           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 657f5ba...3232734. Read the comment docs.

@bocklund bocklund requested a review from richardotis May 29, 2021 22:59
@richardotis
Copy link
Collaborator

I checked this with the six-component steel calculation and the binary phase diagram examples, and it looks like a good change.

@bocklund bocklund merged commit 90696b6 into pycalphad:develop May 30, 2021
@bocklund bocklund deleted the fix-phase-removal branch May 30, 2021 17:45
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

2 participants