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

[WIP] Activating Pyomo linear presolve for IDAES models #1415

Closed
wants to merge 63 commits into from

Conversation

andrewlee94
Copy link
Member

@andrewlee94 andrewlee94 commented May 23, 2024

Fixes None

Summary/Motivation:

This PR builds on previous work to bring the new Pyomo solver interfaces into IDAES and activates the use of the new linear presolver.

All core code (core and models) has been updated to use the presolver, however many parts of models_extra have issues relating to the pre-solver.

  • TSA models: linear presolve results in an "invalid value in AMPL evaluation" exception during initialization. Presovle has been disabled.

  • Tray column and associated models show at least two different initialization failures. Given the current state of these model, this should be addressed by a complete overhaul of the tray column code. Presolve has been disabled.

  • Gas-Solid Contactors: BFB and FB01 have issues with presolve. Disabled presolve.

  • Power Generation:

    • test_NG_PR.py: test_gibbs is poorly written and fails to work (and we are lucky it worked until now). xfailed.

Changes proposed in this PR:

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 license terms described in the LICENSE.txt file at the top level of this directory.
  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.

andrewlee94 and others added 30 commits March 22, 2024 16:38
* Resolve CI failures from new version (1.20.4) of spellchecker (IDAES#1389)

* Empty commit to verify failures on main

* Resolve likely false positives

* Resolve likely true positives

* Resolve probable true positive

* Remove "frst" from typos exclusions

* Remove "ans" from typos exclusions b/c crate-ci/typos#981

* Fix target for 'fv' (flowsheet visualizer) reference (IDAES#1391)

* outputs

* fix hyperlink

* put this back

* put this back2

* replace with simple hyperlink

* Updating black and rerunning it on all files (IDAES#1388)

* Updating black and rerunning it on all files

* Remove duplicated Black requirement

* Update Black rev in pre-commit config

---------

Co-authored-by: Ludovico Bianchi <[email protected]>

---------

Co-authored-by: Ludovico Bianchi <[email protected]>
Co-authored-by: Dan Gunter <[email protected]>
Co-authored-by: Keith Beattie <[email protected]>
@AlexNoring
Copy link
Contributor

@Daison2102 @AlexNoring If you have some time, it would be appreciated if you could look into why the TSA initialization fails with the presolver. It appears that one of the steps results in an invalid value that causes an exception in AMPL (at first guess, I suspect a value is being fixed to a singular point).

For now, I have set the TSA initializer to not use the presolver.

@andrewlee94 The TSA tests pass for me when I set heating_time_guess to 400 for the TestTsaPolystyrene class.

@andrewlee94 andrewlee94 requested a review from mrmundt June 10, 2024 19:30
@andrewlee94 andrewlee94 added the CI:run-integration triggers_workflow: Integration label Jun 11, 2024
@idaes-build idaes-build removed the CI:run-integration triggers_workflow: Integration label Jun 11, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 37.61468% with 68 lines in your changes missing coverage. Please review.

Project coverage is 77.68%. Comparing base (2ac6417) to head (f39ccdf).
Report is 27 commits behind head on main.

Files with missing lines Patch % Lines
...s/subcritical_power_plant/steam_cycle_flowsheet.py 6.06% 31 Missing ⚠️
...rcritical_steam_cycle/supercritical_steam_cycle.py 0.00% 23 Missing ⚠️
idaes/core/solvers/config.py 50.00% 4 Missing ⚠️
idaes/models_extra/column_models/condenser.py 50.00% 1 Missing and 1 partial ⚠️
idaes/models_extra/column_models/reboiler.py 50.00% 1 Missing and 1 partial ⚠️
idaes/models_extra/column_models/tray_column.py 50.00% 1 Missing and 1 partial ⚠️
...d_contactors/unit_models/bubbling_fluidized_bed.py 50.00% 1 Missing and 1 partial ⚠️
...a/gas_solid_contactors/unit_models/fixed_bed_0D.py 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1415      +/-   ##
==========================================
- Coverage   77.80%   77.68%   -0.12%     
==========================================
  Files         393      393              
  Lines       64797    64837      +40     
  Branches    14390    14404      +14     
==========================================
- Hits        50413    50368      -45     
- Misses      11794    11855      +61     
- Partials     2590     2614      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andrewlee94
Copy link
Member Author

Closing in favour of #1436. This PR tried to do too much at once and ran into backward compatibility issues.

@andrewlee94 andrewlee94 deleted the scaling_v2_2 branch June 21, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:High High Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants