Fix Reactor Examples Properties

[bpaul4]

Addresses IDAES/idaes-pse#1376

Correct some property values in the property package used by the CSTR, Plug Flow Reactor, Stoichiometric Reactor and Skeleton Unit Model notebooks.

---

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
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.

---

📚 Documentation preview 📚: https://idaes-examples--95.org.readthedocs.build/en/95/

[bpaul4]

The test failures are expected failures (only Keras examples).

[andrewlee94]

@bpaul4 Where did you find your copy of The Properties of Gases and Liquid (and which edition did you use)? It would be good to get someone to double check the values.

[andrewlee94]

There are a lot of parameters values that I cannot confirm from the references given - either the reference page is incorrect or the values do not match up.

I think this entire property package needs to be completely re-checked to make sure the values and page numbers line up, and some sort of verification testing done to ensure the values it gives are correct.

The examples should also be verified against literature data (which should be available for this case) to ensure the answers are correct. This would likely have revealed that the properties were wrong had it been done when this was first prepared.

[bpaul4]

@andrewlee94 the main changes here are in eg_h2o_ideal.py and egprod_ideal.py. The cstr, plug_flow_reactor, stoichiometric_reactor, and skeleton_unit examples have updates related to the property changes. Some of the supporting files for the notebooks became corrupted due to merge conflicts, including for the equlibrium_reactor and gibbs_reactor examples, and needed to be regenerated to resolve parsing issues from merge conflicts.

[andrewlee94]

I found a a few parameters that are still not correct (there are different correlations for different temperature ranges). You also need to decrease the tolerance on most of your tests in the notebooks.

Finally, in order to make sure we got it right this time, could you add a set of tests for the property packages. If you look in Perry's for example, there are generally columns that show the calculated value of the property at a specific temperature (or temperatures). We should add some simple tests that set up the property package, fix the temperature to that value and make sure the calculated property value is correct. Hopefully you can do this for many of the other sources as well.

You might need to ask @lbianchi-lbl about how best to write these tests so they get run as part of the CI, as I suspect the current workflows only test notebooks (but I might be wrong).

[andrewlee94]

Why are these fixtures and not just module/class level dicts?

[lbianchi-lbl]

FWIW I think the use of fixtures here is appropriate:

• In general, fixtures are the preferred method for sharing data between tests in pytest (see https://docs.pytest.org/en/stable/explanation/fixtures.html)
• In particular, when using mutable data structures such as dicts, using a fixture will ensure a well-defined lifetime for each instance (e.g. if the fixture is defined with scope="class", pytest guarantees that a new instance will be created at the beginning of the test class, and that the same object will be used for all functions within that test class)

[lbianchi-lbl]

(sorry for the "lecturing" tone, I just wanted to provide some motivation for my remarks)

[andrewlee94]

I assume this difference is due to the correction of the property parameters?

[bpaul4]

@andrewlee94 I added cp_mol_liq_comp as Perrys to the config dictionary itself in the properties, which seemed to provide a method for the property to be created. I plan to check the other sources for reference data to validate as many of the remaining properties as possible.

[andrewlee94]

@bpaul4 I do not think we should be adding cp_mol to the supported properties just for the sake of testing - this implies that cp is not required in the main model so we should not be adding it. For testing, you should instead test for what is used - enthalpy. So, instead of writing tests for cp write them instead in terms of entahlpy (h = integral(cp dT))

[bpaul4]

@andrewlee94 I've tested some different ways to approximate the integral manually to calculate cp_mol from enth_mol against the literature values, and the results are close but not within 1e-4. Is there a function within idaes, pyomo, or another package which could provide a more precise calculation of the integral?

[andrewlee94]

@bpaul4 When you say "approximate the integral" what exactly do you mean? If you are referring to the published corrleation you should be able to calculate that analytically, and it you are working from data then the issue is the data and not the integral. Note that any experimental data will have an error in it, and that will generally be on the order of 1% (so a relative tolerance of 1e-4 is unlikely to pass).

Note that this might be a dead-end quest. Depending on what data you have, the answer might be that we need to add cp as a property is we want to verify the results.

[bpaul4]

@andrewlee94 Calculating from the expression directly using the coefficients seems like a redundant test, since we'd basically be checking if a value calculated from the formula in the test equals a value calculated from that same formula in IDAES properties. I used a trapezoid rule between the test points and public data from NIST Webbook.

As you mentioned, it may be better to add cp_mol as a property, or to just set up a test for enth_mol at the reference temperature (298.15 K) since we have those values already.

[andrewlee94]

@bpaul4 What difference/error between the literature data and the model results did you get when using the trapezoid rule? Expecting a tolerance of 1e-4 is unrealistic, but if the error is on the order of 1% then it is probably good.

[andrewlee94]

@bpaul Did you resolve the question about testing enthalpy values and how good (or bad) the comparison between the model and integrated data was?

[bpaul4]

@andrewlee94 An error validation within 1% ended up being appropriate for most of the checks, as you indicated would most likely be the case in your earlier comment. Checking enthalpy directly instead of heat capacity works well and is implemented in the latest commits.

[dallan-keylogic]

Looks good, but consider whether you need to remove heat capacity from the properties supported like @andrewlee94 suggested. At this point, taking it out seems like a waste, but he may have a different opinion.

[dallan-keylogic]

It probably doesn't merit changing here, but we should pretty much always be using log fugacity for phase equilibrium.

[dallan-keylogic]

Weren't we going to remove cp_mol_liq_comp as a property in this package?

[dallan-keylogic]

Remove heat capacity.

[dallan-keylogic]

When we merge a notebook, should it generally be a version with all cells run or a fresh one with no output? It looks like this version hasn't been run (which accounts for all the deletions).

[dallan-keylogic]

Something that's totally inconsequential here but good to know in general:

Testing for membership in Python Sets is faster than testing for membership in Lists. However, Sets only work for hashable objects, which is why Pyomo has a set variant for Vars, Constraints, Params, and Expressions.

[dallan-keylogic]

It's better in general to make phase_list a List instead of a Set. That's because Sets are unordered and we cannot guarantee they are iterated through in the same order every time code is run. Supposedly it's the same in a given Python installation, but in practice I found it can change. This issue caused a really obscure bug when loading solutions from .json files---the phase equilibrium index was getting changed from ("Liq", "Vap") to ("Vap", "Liq") which meant that phase equilibrium variables weren't getting values loaded ultimately leading to solution variables.

All that is irrelevant in this sort of example, but it's best to model best practices in examples so that users make good decisions.