Restructure Unit/Property Model Imports; Eliminate Duplicate Prop Model Names

[kurbansitterley]

Fixes/Resolves:

NA

Summary/Motivation:

Currently importing a property model requires also knowing the name of the actual file the parameter block lives in, which is often non-intuitive and cumbersome. Additionally, we have several property models that have the same named parameter block, which is also non-intuitive and cumbersome (and confusing). The same is true for unit models.

This PR modifies the init so you can:

from watertap.property_models import __________

Or for unit models:

from watertap.unit_models import ________

We also have a few property models that share a name:

• NaClParameterBlock is shared by three property models
• WaterParameterBlock is shared by two property models

This PR also renames them to the following:

• NaClParameterBlock for crystallizer is CrystallizerParameterBlock
• NaClParameterBlock for the temperature dependent property model is NaClTDepParameterBlock
• WaterParameterBlock for the ZO property model is now ZOParameterBlock

This PR also cleaned up and improves the import structure in many unit files to follow

built-in imports

pyomo imports

idaes imports

watertap imports

Imports are updated where necessary. Docs are updated where necessary.

Changes proposed in this PR:

• restructure property package imports
• add all unit models to unit_models init
• eliminate duplicate names of property models to be more intuitive and rename tests to reflect new names
• move ZO property package to property_models and rename to zero_order_prop_pack
• move, improve, and correct ZO property model docs to property model section of docs
• add import headers to all prop model doc files
• update/improve/clean up imports where necessary in entire package
• update docs to reflect all changes

TO DO:

• continue updating imports where necessary
• update docs to reflect changes where necessary

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.

[adam-a-a]

Suggested change

	pair: watertap.core.zero_order_properties;ZOParameterBlock

SHould this pair be updated too?

[kurbansitterley]

yeah docs are still a TO DO

[adam-a-a]

Suggested change

	The ideal water properties module (zero-order property model) contains a simple property package for saline waters which is intended for use with the WaterTap zero-order unit model library. The ideal water property package can be used in a flowsheet as shown below:

Shouldn't we update the description a little bit?

[kurbansitterley]

Yes definitely. I would argue we should drop the "ideal properties" language entirely

[kurbansitterley]

This doc was re-written and moved.

[adam-a-a]

No big deal, but I noticed that in another file change, you switched to importing MaterialFlowBasis from idaes.core. That said, I prefer having MaterialFlowBasis importable from watertap.property_models too for convenience.

[kurbansitterley]

I agree and I was going to do this but thought I would get pushback because it would require doing from idaes.core import MaterialFlowBasis in the init and I am not sure if that is a packaging no-no. But I can add it

[adam-a-a]

just a note on consistency---in other files, you do import ZOParameterBlock--but not here.

[kurbansitterley]

Yeah haven't finished cleaning things up yet. I intend to be consistent across all files.

[adam-a-a]

One issue I thought of with bringing this PR in during the Academy--if we'd like them to bring in any latest changes from main before the course concludes, they'd get whatever those targeted changes are, plus all of this, and I wonder if that'd throw them off (say, if imports start breaking.

[kurbansitterley]

One issue I thought of with bringing this PR in during the Academy--if we'd like them to bring in any latest changes from main before the course concludes, they'd get whatever those targeted changes are, plus all of this, and I wonder if that'd throw them off (say, if imports start breaking.

I am not sure what you mean here -- in theory if this was merged, imports wouldn't be breaking... Also the previous import structure still works fine.

But either way yeah this PR really got away from me, if we want to hold off on it that is ok with me. I would be interested in what real developers think of a PR that modifies >300 files @sufikaur @ksbeattie @dangunter. I kept thinking "this could create a ton of merge conflicts for a ton of people"

[adam-a-a]

One issue I thought of with bringing this PR in during the Academy--if we'd like them to bring in any latest changes from main before the course concludes, they'd get whatever those targeted changes are, plus all of this, and I wonder if that'd throw them off (say, if imports start breaking.

I am not sure what you mean here -- in theory if this was merged, imports wouldn't be breaking... Also the previous import structure still works fine.

But either way yeah this PR really got away from me, if we want to hold off on it that is ok with me. I would be interested in what real developers think of a PR that modifies >300 files @sufikaur @ksbeattie @dangunter. I kept thinking "this could create a ton of merge conflicts for a ton of people"

This can lead to merge conflicts, for one. Two, if you want to be absolutely sure nothing will break, then you should put up a PR on the academy repo that checks that no notebooks will fail when pointing to this PR. Maybe nothing breaks on the academy repo —but you cannot assure that simply because you have tests passing here.

[adam-a-a]

One issue I thought of with bringing this PR in during the Academy--if we'd like them to bring in any latest changes from main before the course concludes, they'd get whatever those targeted changes are, plus all of this, and I wonder if that'd throw them off (say, if imports start breaking.

I am not sure what you mean here -- in theory if this was merged, imports wouldn't be breaking... Also the previous import structure still works fine.
But either way yeah this PR really got away from me, if we want to hold off on it that is ok with me. I would be interested in what real developers think of a PR that modifies >300 files @sufikaur @ksbeattie @dangunter. I kept thinking "this could create a ton of merge conflicts for a ton of people"

This can lead to merge conflicts, for one. Two, if you want to be absolutely sure nothing will break, then you should put up a PR on the academy repo that checks that no notebooks will fail when pointing to this PR. Maybe nothing breaks on the academy repo —but you cannot assure that simply because you have tests passing here.

Here's an expected one to anticipate:

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
Cell In[1], line 12
     10 from watertap.property_models.seawater_prop_pack import SeawaterParameterBlock
     11 from watertap.property_models.NaCl_prop_pack import NaClParameterBlock
---> 12 from watertap.property_models.NaCl_T_dep_prop_pack import (
     13     NaClParameterBlock as NaClParameterBlock_Tdep,
     14 )
     15 from watertap.core.solvers import get_solver

ImportError: cannot import name 'NaClParameterBlock' from 'watertap.property_models.NaCl_T_dep_prop_pack' (my_root\watertap\watertap\property_models\NaCl_T_dep_prop_pack.py)