Skip to content

Update IDAES to 2.1.0 #199

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

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

lbianchi-lbl
Copy link
Contributor

@lbianchi-lbl lbianchi-lbl commented May 11, 2023

Summary/Motivation:

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.md and COPYRIGHT.md 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.

@lbianchi-lbl lbianchi-lbl added the Priority:High High Priority Issue or PR label May 11, 2023
@lbianchi-lbl lbianchi-lbl self-assigned this May 11, 2023
@radhakrishnatg
Copy link
Contributor

@lbianchi-lbl @nareshsusarla @esrawli The test failures in the fossil case study do not appear to be related to the changes in the MultiPeriodModel class. I see initialization and solver errors. So, could you please take a look at that?

@dguittet There is a test failure in the RE case study. I think it has to do with the length of the data being passed to the build_multi_period_model method. I think this should be an easy fix.

@adam-a-a
Copy link
Contributor

@lbianchi-lbl @nareshsusarla @esrawli The test failures in the fossil case study do not appear to be related to the changes in the MultiPeriodModel class. I see initialization and solver errors. So, could you please take a look at that?

@dguittet There is a test failure in the RE case study. I think it has to do with the length of the data being passed to the build_multi_period_model method. I think this should be an easy fix.

One issue seems to occur within test_costing_method in dispatches\case_studies\fossil_case\ultra_supercritical_plant\storage\tests\test_integrated_storage_with_ultrasupercritical_power_plant.py, specifically at initialize_with_costing, where a negative value is being raised to a power:
image

@adam-a-a
Copy link
Contributor

So, the failure mentioned above seems to be attributed (at least initially) to m.fs.hxd.steam_reynolds_number going to negative values.

I should also note that three of these errors already exist when I run tests without any of the changes associated with the IDAES PR.

@lbianchi-lbl
Copy link
Contributor Author

lbianchi-lbl commented May 15, 2023

  • The test failures that are actually related to the changes in Speed up multiperiod model IDAES/idaes-pse#1180 will be addressed here (i.e. on the DISPATCHES side); no further functionality changes are planned on the IDAES PR
  • The other test failures will be investigated separately. @lbianchi-lbl will open another PR against IDAES/idaes:main and start looking for possible commits causing the failures

@radhakrishnatg
Copy link
Contributor

@lbianchi-lbl I ran the tests locally, and test_RE_flowsheet.py is not failing on my laptop. I don't know why those tests are failing here.

@radhakrishnatg
Copy link
Contributor

Based on #202, the remaining RE test failures do not seem to be related to the changes in the IDAES PR.

@lbianchi-lbl lbianchi-lbl changed the title Test changes in IDAES/idaes-pse#1180 Update IDAES to 2.1.0 Jun 5, 2023
@radhakrishnatg
Copy link
Contributor

Hi @lbianchi-lbl Is IDAES 2.1.* compatible with Pyomo 6.5.? If yes, please change the Pyomo version to 6.5.. I'm trying to figure out whether RE test failures are caused by Pyomo version or the IDAES version.

@lbianchi-lbl lbianchi-lbl added Priority:Low Low Priority Issue or PR and removed Priority:High High Priority Issue or PR labels Jun 20, 2023
@lbianchi-lbl
Copy link
Contributor Author

We've decided to use IDAES 2.0/Pyomo 6.5 for the June release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:Low Low Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants