Adding new wastewater treatment modules in BioSTEAM

[yalinli2]

Accidentally created the PR before finish drafting the comment, description added below

This PR is intended to add the new wastewater treatment (WWT) process as described in Li et al in BioSTEAM. As shown in the paper, the new WWT process has the potential to substantially reduce the cost and environmental impacts of wastewater management in biorefineries (through high-rate anaerobic technologies), especially for second-generation biorefineries with lingocellulosic feedstocks.

[yalinli2]

TODOs before formal PR request, aim to complete within the week of 3/14/23

• [X] Add individual WWT unit operations needed in the new process
• [x] Package up so that the entire process and be added to an existing system (like how the existing WWT process works)
• [x] Try the implementation with the corn stover biorefinery and see if the results make sense

[yalinli2]

Hi @yoelcortes , I've implemented the high-rate wastewater treatment system into the cellulosic ethanol system and tested it with the corn stover biorefinery (please use this branch for biosteam and the wwt branch for Bioindustrial-Park):

>>> import biosteam as bst
>>> from biorefineries import cornstover as cs
>>> factor = cs.ethanol_density_kggal

>>> chems = bst.wastewater.append_wwt_chemicals(cs.create_chemicals())
>>> bst.settings.set_thermo(chems)

>>> def get_MESP(sys):
...     if not sys.TEA: cs.create_tea(sys)
...     sys.simulate()
...     MESP = sys.TEA.solve_price(sys.flowsheet.stream.ethanol)*factor
...     print(f'\n\n{sys.ID} MESP: \n${MESP:.2f}/gal')

# With the conventional WWT process
>>> conv_f = bst.Flowsheet('conventional')
>>> bst.main_flowsheet.set_flowsheet(conv_f)
>>> conv_sys = cs.create_system('conventional', WWT='conventional')
>>> get_MESP(conv_sys)
conventional MESP: 
$2.10/gal

# With the high-rate WWT process
>>> highr_f = bst.Flowsheet('high_rate')
>>> bst.main_flowsheet.set_flowsheet(highr_f)
>>> WWT_kwargs = {
... 'flowsheet': highr_f,
... 'process_ID': 6,
... 'autopopulate': True,
... 'mockup': True,
... }
>>> highr_sys = cs.create_system('high_rate', WWT='high_rate', WWT_kwargs=WWT_kwargs,)
>>> get_MESP(highr_sys)
high_rate MESP: 
$1.72/gal

Some of the issues I want to check:

1. There are some chemicals needed for the high-rate process, currently it's appended to the existing chemicals using the append_wwt_chemicals function to adjust the chemicals across the system, there might be a better way to just limit the use of these chemicals to the WWT process (like the Junction class? or use the thermo option of the units?)
2. Some of the unit operations include heat changers as the auxiliary units. I currently has the simulation of these units in the _cost function instead of _run, thinking that we don't need to repeatedly simulate them until the system has converged, is this OK?

Thanks!

[yoelcortes]

@yalinli2, thanks for making so much progress on this recently! It is fine to simulate auxiliary units in _cost. Regarding the wwt_chemicals, I think having the user use add_wwt_chemicals is the best for clarity. You can define as many chemicals as you like and the speed should stay the same.

It should also be OK to add any missing WWT chemicals before creating any units. For example:

def create_wwt_sys():
    chemicals = settings.chemicals
    chemicals = add_wwt_chemicals(chemicals)
    settings.set_thermo(chemicals)
    ...

Both Mixer and MixTank objects can interface between inlet and outlet streams with different chemicals (just like Junction objects). So the code should run well as is because it starts with a mixer.

Thanks!

[yalinli2]

@yoelcortes great! But a little more nuance here, since I need to pre-create the outlets of the system: https://github.com/BioSTEAMDevelopmentGroup/biosteam/blob/4179c1be8ef186503506fbb4725a980bfe57cffb/biosteam/units/wastewater/high_rate.py#L299

So adjusting the chemicals within the system creation function will cause incompatibility issues (as the outlet streams have been created), so I made changes to how the fthermo function works: https://github.com/BioSTEAMDevelopmentGroup/biosteam/blob/e095f9abab7879a81b6d0fd9551b8d2cfc6c3898/biosteam/process_tools/system_factory.py#L221

I also added documentations, etc., and I think it's ready for formal review!

[yalinli2]

@sarangbhagwat FYI for the TAL biorefinery!

[yalinli2]

@yoelcortes I'm not exactly sure why the test failed - the wastewater-related test passed locally

[yoelcortes]

@yalinli2, awesome, all good changes headed in the right direction. I'll take care of it from here. I'm just planning on making changes for compatibility, very minor enhancements, and maybe add a few details here and there.

Thanks,

[yalinli2]

@yalinli2, awesome, all good changes headed in the right direction. I'll take care of it from here. I'm just planning on making changes for compatibility, very minor enhancements, and maybe add a few details here and there.

Thanks,

Wonderful, thanks Yoel!

[yoelcortes]

@yalinli2, the issue with the test just has to do with the reliance on the wwt brach of Bioindustrial-Park (tests are run with the master version). I'll fix the issue tomorrow. I made couple of changes/updates today concerning documentation and code structure:

1) Moved facilities and wastewater out of the biosteam.units (now it is just biosteam.facilities and biosteam.wastewater). Seeing as these depend on biosteam.units and not the other way around I think this should help managing imports without too much cycling through parent dictionaries. 2) Made a high_rate subpackage within biosteam.wastewater. 3) Updated documentation issues. Links to references break when there are multiple of the same number within a module (which was the case for classes and functions in the wastewater module). For some reason readthedocs is not able to generate a the docs well as of right now (the new high_rate and conventional subpackages are missing; https://biosteam.readthedocs.io/en/wwt/API/wastewater/index.html), but generating the docs locally looks good. 4) Now there is no need to add type hints to ins and outs of unit operations. BioSTEAM can take care of these automatically using annotations for Unit subclasses (unless the user adds type hints to the documentation or init annotation themselves). Hopefully this should add consistency in the documentation.

I'll get all tests and documentation passing tomorrow for you to review before merging.

Thanks!

[yoelcortes]

@yalinli2, I'm done with my changes and all tests passing (yay!) except for QSDsan due to a minor issue. Here are some relevant changes:

1) The signature for create_wastewater_treatment_system was changed a little: I opted for using kind instead of WWT for consistency with the way I have set up other system factories. An alternative could also be configuration, but then the other system factories would need to be updated.

2) The create_all_facilities function has been updated back to use WWT as a boolean only. To use a different configuration, pass the 'kind' in WWT_kwargs.

3) biosteam.units.facilities no long works; you might need to update qsdsan to use biosteam.facilities instead.

4) Because of overlap with unit operation in wastewater treatment, none of those unit operations can be imported directly from biosteam. For example, the user must use from biosteam.wastewater.high_rate import SludgeCentrifuge or from biosteam.wastewater.conventional import SludgeCentrifuge, but not from biosteam import SludgeCentrifuge.

5) I made other minor changes to biorefineries which are detailed in the commits, including:

• I opted to use tea instead of TEA for module-level variables (but the Biorefinery.TEA property is fine) because of the confusion that it may be a class. Here is an example:

  from biorefineries import corn
  corn.load()
  # corn.TEA raises AttributeError
  corn.tea # this is OK

• The name parameter for the cellulosic.Biorefinery object is meant to serve as an option for different cellulosic configurations. I understand why only allowing 'corn stover ethanol' is too limiting, but I would still like to make it error to things like 'corn stover biodiesel' (for example). So I added an error if 'ethanol' is not in the name.
• Some cellulosic results changed a little due to updates to chemical properties for more consistency with the Humbird corn stover model.

Once QSDsan is updated, all tests are passing, we can go ahead and merge. Feel free to make any additional changes too.

Thanks!

[sarangbhagwat]

@sarangbhagwat FYI for the TAL biorefinery!

Awesome! I'll definitely be using this for TAL. Thanks for the heads-up, @yalinli2!

[yalinli2]

Thanks @yoelcortes for the updates, on your comments:

1. Looks good to me, kind is actually what I preferred but I was doing it for backward compatibility without updating the biorefineries, it's good that you have all updated.
2. Same as 1.
3. OK and updated.
4. I agree.
5. OK, for the first point, as long as bst.System.TEA still works (i.e., corn_sys.TEA, which I think is the case)

Another thing that bugs me (but I'm not sure of a better solution) is that git throws all history away when you move a module, so all my previous commits on the wastewater module are gone now and it makes it impossible to look back and track previous changes. I'm not sure if there are better ways to do this - I've done the same thing for some of QSDsan's modules for better organization, but I don't like this...

Also, is there a reason that you would change the triple single quotes ''' to triple double quotes """ (i.e., will something not work with ''')? I know you prefer """ but I'm not sure why all my ''' needs to be updated?

Thanks!

[yoelcortes]

@yalinli2, awesome! Thanks for the feedback.

I'm not sure if there is really any safe solution to preserving history of moved files. This sucks, but at least the fact that they existed in another directory and can be traced in the commits helps make the authorship traceable if needed.

Regarding ''', it was just habit of mine to use """ and I kept replacing all as I went through the files. I'll try not to make any useless changes like this to not overpopulate the commits (thanks for pointing this out). You can keep using ''' and I won't change it anymore.

I'll keep taking care of the tests and hopefully merge soon.

Thanks!

[yalinli2]

I'm good with this being merged, thanks @yoelcortes !