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

Generic property package fixes #16

Merged
merged 44 commits into from
Jan 6, 2025
Merged

Generic property package fixes #16

merged 44 commits into from
Jan 6, 2025

Conversation

alma-walmsley
Copy link
Collaborator

@alma-walmsley alma-walmsley commented Dec 24, 2024

Generic property package fixes

  • Switch back to the FTPx state definition

    • EDIT: in the future, if we want to support pure components, we will need to use FPhx. See comments below.
    • On closer inspection, FPhx is just an extension of FTPx by providing enth_mol as a variable. It has the exact same variables and constraints when built, except for an extra variable and constraint in FPhx. This means FPhx will not behave any better than FTPx.
    • Being in FPhx, this increases the complexity of the model for no good reason (+1 variable and constraint). If we want to fix enth_mol, adding a constraint for the expression in FTPx has the same effect.
    • In FPhx, We also have to worry about state bounds for enthalpy.
    • There are no separate initialization methods; they are all built around T and P (recall: T is still a variable in this state definition). But maybe this is not a bad thing? Since we want to be able to use temperature anyway.
  • Increase ITER_MAX for estimating temperature_bubble and temperature_dew

  • Fix solving with constraints instead of state variables

    • We can set the state_vars_fixed kwarg in the generic property initialization routine to True to indicate that all state variables are fixed, so it won't try to fix them, and instead just does a degrees-of-freedom check.
    • Still faced some issues with deactivation and re-activation of constraints during different steps of the initialization routine (ie. our constraint to define a state variable gets deactivated and isn't activated until later, but it's needed earlier for 0 degrees of freedom). Ended up modifying the initialization routine to allow this, and created a PR to idaes: Generic property package: Using constraints to define state variables IDAES/idaes-pse#1554. I do currently have issues with not fixing pressure (ie. using enth_mol / temperature to define pressure), so maybe this requires some more thought.
    • Extended the fix_state_vars method from idaes.core.util.initialization in the modular property package for handling constraints. Applied the same logic to the helmholtz property package and put the method in property_packages.utils.
    • Added a few different tests for these.

@alma-walmsley alma-walmsley marked this pull request as draft December 24, 2024 05:01
@alma-walmsley alma-walmsley marked this pull request as ready for review December 24, 2024 07:38
@alma-walmsley
Copy link
Collaborator Author

alma-walmsley commented Dec 24, 2024

Just a heads up @Mahaki-Leach @bertkdowns you can get some nasty infeasible solves if mole_frac_comp does not sum exactly to 1. At T=~403, P=200000, having 0.33 of argon, oxygen, and nitrogen (sum: 0.99) created this flat line in the T - h curve: It is not a small range... 🙂

sum(mole_frac_comp) == 0.99

image

sum(mole_frac_comp) == 1

image

@bertkdowns
Copy link
Collaborator

bertkdowns commented Jan 3, 2025

this also means FPhx doesn't have any special support for solving pure components #9. The idaes docs seems to contradict this though, so might be worth double checking this actually the case. https://idaes-pse.readthedocs.io/en/latest/explanations/components/property_package/general/state/FPhx.html#application

FPhx is still required, for pure components in the vapour phase region. If your state variable is "water at 100 degrees" you don't know what fraction of it is boiled. You need enthalpy to avoid this problem. I think it's worth the tradeoff of having one more variable and one more constraint, and yeah it's gonna be frustrating having to come up with bounds but we're gonna have to deal with it.

Other than that, mostly sounds good! will talk through it more next week

@alma-walmsley
Copy link
Collaborator Author

From what I have observed FPhx doesn't solve this problem.

  1. Pure components aren't working in either state definition at the moment
  2. FPhx allows you to specify enthalpy, which means you can access the vapor-phase region. But so does our definition of FTPx, since we support adding a constraint for enthalpy... which has the same effect. Having enthalpy as a variable instead of an expression does not improve things; if anything, it makes it harder for the solver due to more complexity. It would be different if we had FPhx with enthalpy as a variable and temperature as an expression, but this is not the case.

Could be wrong, but we won't know unless we can get pure components to work.

@alma-walmsley
Copy link
Collaborator Author

If the user constrains enthalpy to a value within the vapor-phase region, then the solver can't adjust temperature to get that enthalpy, since the same temperature value corresponds to many enthalpy values on that line.

If we're in FPhx, don't we have the same problem? The solver still has to find a valid temperature for that enthalpy, since temperature is a variable.

@bertkdowns
Copy link
Collaborator

finding a valid temperature for that enthalpy is easy though, isn't it? there's only one correct temperature. It's when you're trying to find a valid enthalpy from a temperature that you get problems, as there's multiple valid enthalpies.

I'll try make an idaes flowsheet with a single component in the phase region with ftpx and fphx and compare

@alma-walmsley
Copy link
Collaborator Author

Ohhh, I see what the issue is here. It's impossible to have a defined model in the vapor-phase region using state variables T and P (can think of it as, "what value of the state variables corresponds to vapor_frac=0.5?", which is actually impossible in FTPx)

  1. Probably wait until we can get pure components to work (it isn't much to change but I wouldn't switch unless it's needed)
  2. I want to see if a state definition can be created with T as an expression

@bertkdowns
Copy link
Collaborator

yeah you can create a state definition with T as an expression, but from my understanding of our conversations with Andrew, it's best not to. Having temperature as a variable allows the solver to "relax"/change it a little bit and might avoid the problems we had with smooth temperature.

I'm pretty sure I remember andrew saying "temperature should always be a variable". And I'm pretty sure I remember him saying "Yay!" when we said we were gonna use fphx too 😄

@alma-walmsley
Copy link
Collaborator Author

alma-walmsley commented Jan 4, 2025

I tend to disagree, more complexity is usually worse, and "allowing the solver to "relax"/change it a little bit" probably just means more unpredictability. That said, I have no idea how to make temperature an expression; it seems baked into the depths of the smooth VLE constraints...

Edit: Generic property packages says that temperature needs to be a variable. So I don't think we're getting around this one...

@Mahaki-Leach Mahaki-Leach merged commit ce4290f into main Jan 6, 2025
2 checks passed
@Mahaki-Leach Mahaki-Leach deleted the fix-fphx branch January 6, 2025 00:59
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.

3 participants