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

add steam heater and condenser #1358

Merged
merged 31 commits into from
Jul 2, 2024

Conversation

ElmiraShamlou
Copy link
Contributor

@ElmiraShamlou ElmiraShamlou commented Apr 12, 2024

Fixes/Resolves:

Adds a steam heater and condenser unit model, assuming full condensation at the outlet.
Estimate cooling water flow for the condenser option assuming specific design delta T.
Compatible with the WaterTap Property Packages.
Evaporative based desalinations like MD flowsheet, and crystallizer flowsheet will benefit from this added steam heater and condenser unit models. 

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
Copy link
Collaborator

@ElmiraShamlou @adam-a-a I am guessing you aren't aware of this, but IDAES already has a model for a heat exchanger with a condensing section. In order to avoid duplication of code and effort, it would probably be good idea to check this out to see if it might meet your needs: https://github.com/IDAES/idaes-pse/blob/main/idaes/models_extra/power_generation/unit_models/feedwater_heater_0D.py

@ElmiraShamlou
Copy link
Contributor Author

@ElmiraShamlou @adam-a-a I am guessing you aren't aware of this, but IDAES already has a model for a heat exchanger with a condensing section. In order to avoid duplication of code and effort, it would probably be good idea to check this out to see if it might meet your needs: https://github.com/IDAES/idaes-pse/blob/main/idaes/models_extra/power_generation/unit_models/feedwater_heater_0D.py

@andrewlee94 Thanks for the comment. Yes, I'm aware of the IDAES feed water heater. The current PR aims to enhance the steam heater's compatibility with the WaterTAP property packages. This is because the existing IDAES steam heater requires properties that are not available in WaterTAP property packages. We have previously discussed the possibility of integrating these properties to simplify future development efforts with the team.

@adam-a-a
Copy link
Contributor

adam-a-a commented Apr 17, 2024

I am unsure whether the Mac failure is something we want to just dodge with requires_idaes_solver or if this is something that we deem fixable. It appears that something is going wrong with the report method with a NoneType where a path is expected. @bknueven I am guessing you'd recall

@ElmiraShamlou
Copy link
Contributor Author

I am unsure whether the Mac failure is something we want to just dodge with requires_idaes_solver or if this is something that we deem fixable. It appears that something is going wrong with the report method with a NoneType where a path is expected. @bknueven I am guessing you'd recall

@adam-a-a I believe it's because of Chen's approximation

@bknueven
Copy link
Contributor

I am unsure whether the Mac failure is something we want to just dodge with requires_idaes_solver or if this is something that we deem fixable. It appears that something is going wrong with the report method with a NoneType where a path is expected. @bknueven I am guessing you'd recall

The issue here is we call some external function not available without IDAES extensions. We should just use requires_idaes_solver.

@lbianchi-lbl we should probably consider updating the macOS tests given these external functions are now available for macOS, although that would require re-working the requires_idaes_solver handle to be a bit more subtle (really should be ipopt_has_HSL_solvers...

@ElmiraShamlou
Copy link
Contributor Author

ElmiraShamlou commented Apr 19, 2024

My main comment is that I'm not convinced about introducing a new unit model to replicate the 0D heat exchanger with additional saturation constraints. In my previous experience with power plant modeling, you could simply build a heat exchanger and impose additional performance constraints. As long as you have good initial values, the optimization should work fine. Please correct me if the steam heater model includes more than that.

@Morgan88888888 The primary reason for developing this unit model is to calculate the steam consumption rate (given complete condensation of the steam at the outlet) across various flowsheets and configurations in treatment systems like membrane distillation where the steam requirement significantly varies with design. Given that the hot inlet steam flow rate is variable, additional steps are added to the main heat exchanger's initialization method to improve initialization for this specific case. From my experience, initializing a steam heater in the treatment flowsheet without these additional steps was challenging. If you have experience using heat exchangers in similar applications without the need for modifications in initialization, sharing your experience could be greatly helpful.  Another consideration is that we plan to add flowsheets containing multiple steam heaters; having this unit prevents the need to replicate extra constraints for each one.

@bknueven
Copy link
Contributor

Another consideration is that we plan to add flowsheets containing multiple steam heaters; having this unit prevents the need to replicate extra constraints for each one.

I strongly agree with this -- as soon as we have the same code in two different places a refactor with tests like this makes the most sense for maintainability and reliability.

Copy link

codecov bot commented May 15, 2024

Codecov Report

Attention: Patch coverage is 97.80220% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.65%. Comparing base (2c7f7d8) to head (6608eeb).
Report is 1 commits behind head on main.

Files Patch % Lines
watertap/costing/unit_models/heat_exchanger.py 80.00% 1 Missing ⚠️
watertap/unit_models/steam_heater_0D.py 98.83% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1358      +/-   ##
==========================================
+ Coverage   93.63%   93.65%   +0.01%     
==========================================
  Files         285      286       +1     
  Lines       30580    30670      +90     
==========================================
+ Hits        28635    28723      +88     
- Misses       1945     1947       +2     

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

@ElmiraShamlou ElmiraShamlou changed the title add steam heater add steam heater / condenser Jun 6, 2024
@ElmiraShamlou ElmiraShamlou changed the title add steam heater / condenser add steam heater and condenser Jun 6, 2024
@ElmiraShamlou ElmiraShamlou marked this pull request as ready for review June 27, 2024 20:05
Copy link
Contributor

@MarcusHolly MarcusHolly left a comment

Choose a reason for hiding this comment

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

LGTM - just one comment that may or may not hold up this PR

@MarcusHolly MarcusHolly added the 🍒 Merge to rel Merge commit(s) from this PR to release branch label Jul 1, 2024
@ElmiraShamlou ElmiraShamlou requested review from Morgan88888888 and removed request for Morgan88888888 July 2, 2024 13:14
Copy link

@Morgan88888888 Morgan88888888 left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍 Thanks for your work on this.

@lbianchi-lbl lbianchi-lbl merged commit e592f86 into watertap-org:main Jul 2, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 Merge to rel Merge commit(s) from this PR to release branch Priority:Normal Normal Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants