Skip to content

Fix bugs related to flow_fractions - #258

Merged
yoelcortes merged 2 commits into
masterfrom
fix-flow-fractions
Oct 8, 2026
Merged

yoelcortes merged 2 commits into
masterfrom
fix-flow-fractions

Conversation

@yalinli2

@yalinli2 yalinli2 commented Oct 7, 2026

Copy link
Copy Markdown
Member

Two changes:

  1. For the newly added define_credit function, the doc says it can accept a function, but it couldn't, so I added a helper function for that.
  2. For the Unit._flow_fractions attribute, it wasn't included in _init_results function, though other dict were included, It causes problems in qsdsan since it doesn't call Unit.__init__ but _init_results. If Unit._flow_fractions shouldn't be added to _init_results, I can patch up qsdsan, thanks!

@yoelcortes yoelcortes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks reviewing recent commits and improving the code!!

@yoelcortes
yoelcortes merged commit 09e4b28 into master Oct 8, 2026
1 of 3 checks passed
@yoelcortes

yoelcortes commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

@yalinli2, I just realized that the Unit._init_results method is not used in BioSTEAM. A long time ago, we used this method within Unit.__init__. Later, I moved the initialization of results container variables to Unit.__init__ so that Sphinx can automatically add documentation left in #: comments (Sphinx only looks at variables in init).

Would it be better to move the Unit._init_results method to QSDsan?

By the way, I have not been running QSDsan and making sure my changes are backwards compatible (sorry for the trouble!). I will start doing so again with future commits.

Also, for any changes to core classes (e.g., Unit, Stream, TEA, System; not specific unit operations), I will plan on submitting a pull request for your review. Let me know if you have any specific BioSTEAM classes that would be helpful to keep track of with PRs (as opposed to direct commits on my side).

Thank you!

@yalinli2

yalinli2 commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Thanks @yoelcortes ! I've updated QSDsan so it doesn't call Unit._init_results, you can just remove it if BioSTEAM doesn't use it. Same for `Unit._init_utils'.

Sounds good on the test and PR review!

@yalinli2
yalinli2 deleted the fix-flow-fractions branch October 8, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants