CalebBell / thermo

Thermodynamics and Phase Equilibrium component of Chemical Engineering Design Library (ChEDL)
MIT License
594 stars 114 forks source link

Proposal on lists/numpy array support #137

Open yoelcortes opened 10 months ago

yoelcortes commented 10 months ago

@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 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):

  1. 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.
  2. 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!