You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I noticed that you've made some efforts to support both lists and numpy arrays through the use of the scalar attribute for EOS and Phase classes. I think it is a nice idea to cater to both options. The downside is that this adds development overhead. For example, the flash algorithms do not yet use array operations and assume zs are lists. There are also places in Phase and EOS classes were array operations could be used when scalar is False. Another downside is the possibility that scalar is False yet there are lists being used as arrays which go unnoticed, slowing up the code. Code may need to be added to avoid this issue.
Based on these thoughts, I propose the following options (the first being my preference):
Drop support for arrays and make sure everything is a list. CPython itself is becoming much faster and I believe the main use cases for thermo won't exceed hundreds of components (where contiguous array operations would excel). Most of the code is optimized for lists, so this would also be a simple change.
Continue with the intent to support arrays. In which case, I'd also like to force consistency with compositional relationships, such as all phases (and eos objects) within a flash must use either only arrays or only lists. I would also recommend renaming scalar to something like use_lists (at first impression I thought scalar meant a single component).
Looking forward to reading your thoughts on this,
Thanks!
The text was updated successfully, but these errors were encountered:
@CalebBell,
I noticed that you've made some efforts to support both lists and numpy arrays through the use of the
scalar
attribute for EOS and Phase classes. I think it is a nice idea to cater to both options. The downside is that this adds development overhead. For example, the flash algorithms do not yet use array operations and assumezs
are lists. There are also places in Phase and EOS classes were array operations could be used whenscalar
is False. Another downside is the possibility thatscalar
is False yet there are lists being used as arrays which go unnoticed, slowing up the code. Code may need to be added to avoid this issue.Based on these thoughts, I propose the following options (the first being my preference):
scalar
to something likeuse_lists
(at first impression I thoughtscalar
meant a single component).Looking forward to reading your thoughts on this,
Thanks!
The text was updated successfully, but these errors were encountered: