BioSTEAMDevelopmentGroup / biosteam

The Biorefinery Simulation and Techno-Economic Analysis Modules; Life Cycle Assessment; Chemical Process Simulation Under Uncertainty
Other
176 stars 35 forks source link

Unit group improvements #147

Closed sarangbhagwat closed 1 year ago

sarangbhagwat commented 1 year ago

See issue 'Unit group metric fractions do not add to 100%'.

This adds an argument 'scale_to_positive_values', set to True by default to retain original behavior.

Before:

df_TEA_breakdown = bst.UnitGroup.df_from_groups(
    unit_groups, fraction=True,
)

print(df_TEA_breakdown)

Output: Net electricity production 0 100 100 -2.92 200 -3.55 300 -0.808

After:

df_TEA_breakdown = bst.UnitGroup.df_from_groups(
    unit_groups, fraction=True,
    scale_fractions_to_positive_values=False
)

print(df_TEA_breakdown)

Output: Net electricity production 0 108 100 -3.14 200 -3.83 300 -0.872

yoelcortes commented 1 year ago

@sarangbhagwat, this is a nice addition and thanks for the typo correction. Could you add the parameter to the docstring, add a test (either in the docstring or as a test function in the tests folder), and make sure all tests are passing? I'll get it merged as soon as this gets done.

sarangbhagwat commented 1 year ago

Sure thing! I just added both.

yoelcortes commented 1 year ago

Nice, @sarangbhagwat, I think it is almost ready. Here is what is left:

The first point is what is breaking the tests. Before committing, you can check if tests are passing locally with:

pytest . -v --disable-numba=1

I really appreciate the contribution, Thanks!

sarangbhagwat commented 1 year ago

Fixed the docstring (and the one for UnitGroup, which was failing due to the "Materiral" typo that was fixed in my earlier commit). Thanks!

yoelcortes commented 1 year ago

@sarangbhagwat, great! No worries on the error in the tests. It has to do with IDs not matching. I'll take care of it soon and merge.

Thanks again!

yoelcortes commented 1 year ago

@sarangbhagwat, I accidentally included changes that relies on thermosteam's show branch which is currently under review. All tests are passing using that branch. I'll wait until that gets merged before merging this branch.