BioSTEAMDevelopmentGroup / Bioindustrial-Park

BioSTEAM's Premier Repository for Biorefinery Models and Results
MIT License
38 stars 18 forks source link

Refactoring and documentation for cornstover and cane biorefineries #53

Open yoelcortes opened 1 year ago

yoelcortes commented 1 year ago

I think it is a little difficult to navigate imports and dependencies between biorefinery submodules (e.g., cornstover, sugarcane, lipidcane, and oilcane). Also, although the readme gives basic instructions for how to use the biorefineries, there is no documentation for process developers seeking to use only biorefinery sections. Here is a list of issues regarding organization and documentation:

Importing Issues:

Documentation issues:

I'm thinking about moving some parts from these submodules (i.e. cornstover, sugarcane, lipidcane, and oilcane) into new modules called cellulosic, ethanol, and biodiesel. Additionally, I'm also planning on creating a new cane submodule where all cane biorefineries can be loaded from. Here is a layout of what this would look like:

biorefineries/
- corn/
- cornstover/
- cane/
   - juicing.py
   - sugarcane/
     - juice_to_ethanol.py
     - juice_bagasse_to_ethanol.py
     ...
   - oilcane/
     - juice_to_biodiesel_ethanol.py
     - juice_bagasse_to_biodiesel_ethanol.py
     ...
- cellulosic/
- ethanol/
- biodiesel/
...

The cornstover will import from ethanol and cellulosic, cane will import from cellulosic, ethanol, and biodiesel, and corn will import from ethanol.

Keeping this layout in mind, I plan on adding biorefineries as a submodule in the biosteam repository (similar to thermosteam) and add detailed documentation of each biorefinery in the biosteam readthedocs. So in the API Reference there will one more section called Biorefeneries.

To ensure backwards compatibility without changing any imports, the sugarcane, lipidcane, and oilcane submodules will be "convenience" submodules that import from cane and works the same as before (so no tests need to be changed).

I'll be working on this in the cane branch. @yalinli2, @sarangbhagwat, @daltonwstewart, I'll request your review later so that you can double check that nothing breaks on any of the biorefineries you are working on. I won't be changing any variable/function names, just move things around for more clarity and documentation.

Thanks!

yalinli2 commented 1 year ago

Sounds great, thanks @yoelcortes I was going to suggest whether we want to be more process-specific (e.g., instead of cellulosic, we have preatreatment, or dilute_acid_pretreatment), but then I recall you have similar modules like create_dilute_acid_pretreatment_system, so your current plan sounds good!