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

Replace domain comparison by type(domain) comparison #996

Merged

Conversation

RomeoV
Copy link
Contributor

@RomeoV RomeoV commented May 2, 2019

Fixes #995
Note that this bug is blocking development for #983

Fixes #995

Summary/Motivation

Variables of a cloned models don't get fixed by the Transformation (due to an ID copy issue). See the assigned issue for more information.

Changes proposed in this PR:

  • change domain comparison to type(domain) comparison

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@RomeoV RomeoV mentioned this pull request May 2, 2019
5 tasks
Copy link
Member

@jsiirola jsiirola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no issue with patching this transformation until we resolve #326, however I think I would prefer to make the "patch" part clear so that it can be removed once #326 is resolved.

pyomo/core/plugins/transform/discrete_vars.py Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented May 2, 2019

Codecov Report

Merging #996 into master will increase coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #996      +/-   ##
==========================================
+ Coverage   69.72%   69.84%   +0.11%     
==========================================
  Files         462      462              
  Lines       72194    72494     +300     
==========================================
+ Hits        50338    50630     +292     
- Misses      21856    21864       +8
Impacted Files Coverage Δ
pyomo/core/plugins/transform/discrete_vars.py 76.36% <100%> (ø) ⬆️
pyomo/common/errors.py 100% <0%> (ø) ⬆️
pyomo/contrib/fbbt/fbbt.py 96.12% <0%> (+0.36%) ⬆️
pyomo/contrib/gdpopt/cut_generation.py 91.33% <0%> (+1.57%) ⬆️
pyomo/contrib/gdp_bounds/compute_bounds.py 91.04% <0%> (+1.91%) ⬆️

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 ea16161...72dbaba. Read the comment docs.

@jsiirola jsiirola merged commit 56cab09 into Pyomo:master May 7, 2019
@RomeoV RomeoV deleted the 995-TransformationFactory-core-fix_discrete branch May 7, 2019 22:48
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.

TranformationFactory('core.fix_discrete') doesn't work for cloned models
6 participants