Generic Property Package: Problems with changing State Definition from FTPx to FPhx

[bertkdowns]

I'm trying to figure out how to modify a Generic Property Package to work with different state definitions. I've got a simple flowsheet just trying to solve a heater, using a benzene-toluene mixture with ChemSep data.

The property package works fine with the state_definition:FTPx, but doesn't work with FPhx at all.

I'm unsure what these errors mean, what could I do to look into them further? I've looked at the ceos.py file in idaes and as far as I can tell, PR_am should be created by the common() method. This method is called in the generic_property.py file, as part of the GenericStateBlockData build() method:

However, the problem seems to arise because PR_am is used in the define_state() method called just above on line 2419. - before the property has even been added.

Another part of the error message seems to say it's a misconfigured property set,

Is this a bug, or is it due to some misconfiguration of my state definitions & property packages?

The script i'm building is avaliable here:
https://github.com/bertkdowns/model-predictive-control/tree/8e38e1b3323b3baea08ae94b8c141daa4e7150b7/testing_peng_robinson

and can be run with the following commands

git clone https://github.com/bertkdowns/model-predictive-control.git
cd model-predictive-control/testing_peng_robinson
python solve.py

tested using idaes-pse version 2.5.0 on python 3.11. It runs fine until you change "FTPx" for "FPhx" in the configuration.py file.

[andrewlee94]

@bertkdowns At first glance, it appears to be due to things occurring out of order as you suggested. For some reason PR_am is being called for in the define_state method. Looking at the traceback, it looks like it is occurring when the model tries to build enth_mol_phase.

I suspect this might be an edge case we did not anticipate, and it might take a bit of thought about how to fix it. I would start by looking throguh the define_state method to see why it is calling for enth_mol_phase, and see if there is a way to defer that.

[bertkdowns]

The reason it calls enth_mol_phase is because define_state adds a constraint that enth_mol == sum of enth_mol_phase. If I move that logic from define_state() to the common() method of CEOS.py it seems to work:

However, this probably would break other state definitions.

The other way I can think of is to define enth_mol_phase to return a Var(), and a constraint could be added to it in common() so it maintains the same value as the current Expression:

I did try this but had some errors as it was treating the Var as an AbstractVar, but it should be possible.

Or the last option is to modify either build_on_demand() or enth_mol_phase() to check if common() has been called and call common() earlier to initialise the block. (at that point, maybe it'd be easier to get rid of the common() method and just use build_on_demand() to create all the properties, rather than needing to initialise some things first.) Though not sure if build_on_demand supports building Vars(), Sets, and Constraints or not.

If you could give me some guidance on how to fix it, I'd be happy to do so and add a PR.

[andrewlee94]

OK, this is an edge case we had not thought of. However, I think there is an easy solution. I think the answer lies in moving the creation of the constraint enth_mol == sum of enth_mol_phase from the state definitions (I did quick check and it appears in two of them) to the build method in idaes/models/properties/modular_properties/base/generic_property.py (not the common method, as each EoS has its own method so we would need to remember to update all of these).

I suggest trying to add the following at line 2107 in generic_property.py:

# Check to see if state definition uses enthalpy
if self.is_property_constructed("enth_mol"):
    # State definition uses enthalpy, need to add constraint on phase enthalpies
    @self.Constraint(doc="Total molar enthalpy mixing rule")
    def enth_mol_eqn(b):
        return b.enth_mol == sum(
            b.enth_mol_phase[p] * b.phase_frac[p] for p in b.phase_list
        )

You should then be able to remove the constraint definition from FPhx.py and FcPh.py in the state definitions folder. You can test to see if this works by running the IDAES test suite using pytest from the root idaes folder (it will take a half hour or so). I suspect you will see some failures related to the state definitions as the constraint will have been moved, but we can fix those later.

[bertkdowns]

Okay, I've tried to do that. The following tests are failing:

As far as I can tell it is to do with the enth_mol_equation:

The Unattached VarData appears to be the phase_frac property is missing. It's only created in define_state in FPhx, FcPh, etc. I'm not sure if this will cause errors when solving or not, or if it's okay to define it later.

It seems that it's defined later partly because it's ambiguous what it's referring to.

Should I try refactor this into mol_phase_frac and mass_phase_frac, or will this cause too many breaking changes?

[andrewlee94]

This looks like it is a bit harder than we had hoped, so I've opened an issue and will try to find someone with some time to look at it (it will probably be a few weeks at least as we are busy at the moment).

[andrewlee94]

@bertkdowns Sorry it took a while to get back to this, but I managed to sort out the issue. The original fix I suggested was in fact correct; the test failures you saw were due to obscure issue in how the tests were written rather than the fix itself. I just opened a pull request (#1505) to make the changes and fix the tests.

[bertkdowns]

Awesome, glad it worked easier than expected. I can confirm that it's working for me now too.