Pressure changer initialization guesses

[alma-walmsley]

Fixes # .

Summary/Motivation:

The pressure changer initialization routine does not use the normal control volume initialization routine. I assume this is because it needs to take into account ratioP for its outlet pressure guess? See d15028f.

The pressure changer initialization differs in that it sets up guesses for the outlet state before the inlet state is initialized. This means if the values of the inlet state vars were to change during the inlet initialization, the guesses for the outlet state are now incorrect. (The control volume initialization does not have this problem since it estimates the outlet state after inlet state initialization).

This occurs for me since I am setting constraints to "define" the state variables. The inlet guess may well be the default value, and change during initialization to conform to a constraint. Which is fine, except that the same (stale) inlet guess is used when initializing the outlet state block, and it may be fixed there... leading to problems in later stages of initialization.

Instead, the outlet guesses should be created from the inlet state after the inlet state is initialized.

Changes proposed in this PR:

• Move setting guesses in state_args_out to after the properties_in initialization
• Load guesses directly from the current inlet properties (taking into account deltaP/ratioP) - instead of using the guesses that were used previously.
• Apply the same logic to both init_isentropic and init_adiabatic

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.

[alma-walmsley]

I realise after writing up this PR it may be easier (and more maintainable?) to revert back to using the normal control volume initialization. I think the control volume initialization would just need to be modified to take into account ratioP when estimating the outlet state...?

Some logic would still need to be kept for the properties_isentropic guesses.

[andrewlee94]

@alma-walmsley My first observation here is that you are using the old initialization API which is on the path to deprecation and that it would be better to switch to the new Initializer approach. The same issues may appear there as well, but that is where long term support will occur.

Also, it might help to clarify what you mean by "This occurs for me since I am setting constraints to "define" the state variables. "; are these constraints being added to you pressure changer directly? If so, then this violates some of the assumption that went into the design of the initialization routine, i.e., that the inlet conditions would be fixed by something upstream and would have the correct/desired values and thus can be used directly (that said, the first step really should be to initialize the inlet state before projecting the outlet to handle different sets on state variables).

[alma-walmsley]

it might help to clarify what you mean by "This occurs for me since I am setting constraints to "define" the state variables. "

This is the same as mentioned in #1554, but I will add it here for clarity:

Suppose we know the value of the flow_mass (but not flow_mol) going into a compressor:

m.fs.compressor = Compressor(property_package=m.fs.properties)
m.fs.compressor.isentropic_efficiency[0].fix(1)
m.fs.compressor.control_volume.properties_in[0].temperature.fix(280)  # K
m.fs.compressor.control_volume.properties_in[0].pressure.fix(100000)  # Pa
m.fs.compressor.control_volume.properties_out[0].pressure.fix(200000)  # Pa

# add a constraint for flow_mass to the state block
m.fs.compressor.control_volume.properties_in[0].add_component(
    "flow_mass_constraint", Constraint(rule=m.fs.compressor.control_volume.properties_in[0].flow_mass == 10)  # kg/s
)

The IDAES initialization routine usually expects the state var flow_mol to be fixed (or try to fix it), but we have got around that by extending the state block and overriding the initialization routine to not fix flow_mol. So can initialize the inlet state block which gives us the correct value for flow_mol based on the flow_mass_constraint. So far so good.

However, the guesses provided to the outlet state block are determined before the inlet state is solved. So the correct flow_mol value that the inlet initialization solved for, is not used as a guess in the outlet initialization (it uses whatever value was in flow_mol before starting initialization. Note that there is no issue if we used flow_mol instead of flow_mass, since the state variable remains the same).

that said, the first step really should be to initialize the inlet state before projecting the outlet to handle different sets on state variables.

Exactly this, which is what is done in this PR (but not handling different state variables). Which the control volume initialization handles properly.

I will also have a look at the new Initializer approach and see what that has to offer. Thanks 🙂

[andrewlee94]

Given my comment on the other PR, I would suggest trying out the new Initializers as they are the path forward. As I mentioned, model.initialize is slated for eventual deprecation so it is not worth spending a lot of time fixing something that will go away before too long. Additionally, Initializers have some additional features that might make this easier if you go down the plug-in route I mentioned in the other PR.

[ksbeattie]

@alma-walmsley, you should be able to fix the black failure by just running black in your branch (and the other PR of yours).

[lbianchi-lbl]

Considering the more "discussion-y" nature of this PR, I'm converting it to Draft for the time being.

[alma-walmsley]

Actually I think it is ready for review; I will try to explain in depth what is going on here:

We are currently using compressor.initialize() to initialize the compressor model. I think we will eventually switch to the Initializer approach, but that is not the case at the moment. However, the root of the problem really lies in the init_isentropic / init_adiabatic methods, which this PR aims to fix, instead of writing some workaround/custom logic with the new Initializer approach.

The root of the problem is the order that things are done:

1. pass state variable values to the outlet
2. initialize the inlet state
3. initialize the outlet state
4. ... (initialize the isentropic block, solve for isothermal conditions, solve the whole model)

This PR changes the order so that:

1. initialize the inlet state
2. pass state variable values to the outlet
3. initialize the outlet state
4. ... (initialize the isentropic block, solve for isothermal conditions, solve the whole model)

Why is this a problem?

If the inlet state variables change during inlet state initialization, then the guesses that were passed to the outlet are incorrect (since these guesses were passed beforehand).

In our case, the value of an inlet state variable would change if the user does not know the value of it, but does know the value of one of the expressions on the state block. The state variable is unfixed, and a constraint is added for an expression.

For example, if enth_mol is a state variable, and the user knows the value of temperature (an expression) but not enth_mol, then enth_mol is unfixed and a constraint is added for temperature to the state block.

However, this breaks the rules of initialization - that all state variables should be fixed. Instead, a guess for enth_mol should be fixed during initialization, and the constraint for temperature applied afterward (ie. at the flowsheet or after initializing the unit model).

However, this will cause problems with a bad enth_mol guess, or at least the solver will have to do more work. This can be improved by keeping the constraints at the state block level:

• Deactivate constraints on the state blocks before initializing the unit model, and fix the state variables as normal.
• Override the state block initialize method:
  • Run the original state block initialization as normal
  • Solve the state block with constraints instead of state variables (if applicable)
  • Deactivate the constraints and fix the state variables.
  • Finish state block initialization.
• The unit model initialization always sees the state variables as fixed, but may see a change in the state variable values.

The reasoning behind doing it this way is because it is easier to solve a smaller model (ie. a single state block) rather than something more complex (ie. a unit model).

Whether you agree with the above approach or not (which describes how the issue appears in our case), doesn't really affect the validity of this PR. It will not affect anything if the inlet state variables remain constant during initialization. And why shouldn't I be able to use a custom state block that changes the values of the state variables, but maintains them being fixed (for the unit model level)?

This problem is unique to the init_isentropic / init_adiabatic methods; the control_volume initialization does pass guesses in the right order. If nothing else, it is more consistent with control_volume initialization.

@dallan-keylogic @lbianchi-lbl @bertkdowns

[dallan-keylogic]

If the inlet state variables change during inlet state initialization

The problem is that everything about IDAES has been designed so that, if inlet state variables are fixed and a certain set of process variables are fixed, then the unit model is square and can be solved independently. This features not just in initialization, but also the way information is passed between unit models using arc constraints. Ideally, the user should only be specifying information about the state variables in Feed blocks (although often that gets violated, especially in processes with multiple recycle loops). So long as you can solve a Feed block, all information about state variables can be propagated along arcs and you don't have to worry about converting from whatever the user tells you.

I'm not sure exactly how you're deviating from this workflow (except possibly by not including Feed blocks, because that's pretty common). The general solution, it seems to me, is to create a state block Initializer that works for whatever set of variables you allow the user to specify, then use it to solve for the actual state variables at the point where you're letting the user specify information, before calling the IDAES initialization routine as normal.

I'm willing to consider merging this PR if it's helpful in the short term, because it doesn't appear to cause any issues for people using the conventional IDAES workflow. However, I would strongly recommend bringing your process in line with what I outlined, because I'm not willing to spend time and effort supporting a workflow designed to avoid specifying state variables.

[dallan-keylogic]

For clarification, I'm suggesting a workflow like:

def CustomInitializer(InitializerBase):
    def initialization_routine(self, model: Block):
         ... Code to initialize Block in desired way...

...
user_input = get_user_input()

m.fs.dummy_state_block.activate()

for t in m.fs.time:
    for var, val in user_input[t]:
        var_obj = getattr(m.fs.dummy_state_block[t], var)
        var_obj.fix(val)

state_init_obj = CustomInitializer()

state_init_obj.initialize(m.fs.dummy_state_block)

for t in m.fs.time:
    for var_obj in m.fs.dummy_state[t].define_state_vars():
        for idx in var_obj:
            # If var_obj is not indexed, then there is only a single key: None
            var_name = var_obj.local_name
            new_var_obj = getattr(m.fs.unit_model.control_volume.properties_in[t], "var_name")
            new_var_obj.fix(var_obj.value)

# This could be a single step, but I want to be clear about what these methods are doing
unit_init_class = m.fs.unit_model.default_initializer
unit_init_obj = unit_init_class()

unit_init_obj.initialize(m.fs.unit_model)

Let me know if you have comments or questions about this. I'd be interested in hearing @andrewlee94 's take on things, if he's around and has any spare time.