BioSTEAMDevelopmentGroup / thermosteam

BioSTEAM's Premier Thermodynamic Engine
Other
57 stars 12 forks source link

Minor tweak to allow multiple Indexer #45

Closed yalinli2 closed 2 years ago

yalinli2 commented 2 years ago

Hey @yoelcortes, the way that currently thermo data is cached, as in below where the thermo condition is used as the key to the _data_cache dict, assumes that the user is always retrieving volume info from the cache dict

https://github.com/BioSTEAMDevelopmentGroup/thermosteam/blob/4ccd24bc14a898bd696123bf6a46a0303e55e549/thermosteam/indexer.py#L1043

Would it be better to change it into a nested dict like

    try:
        vol = self._data_cache['vol'][TP]
    except:
        chemicals = self.chemicals
        mol = self.data
        vol = np.zeros_like(mol, dtype=object)
        for i, chem in enumerate(chemicals):
            vol[i] = VolumetricFlowProperty(chem.ID, mol, i, chem.V,
                                            TP, None, self._phase)
        self._data_cache['vol'] = {}
        self._data_cache['vol'][TP] = \
        vol = ChemicalVolumetricFlowIndexer.from_data(property_array(vol),
                                                      self._phase, chemicals,
                                                      False)

I noticed this because in QSDsan, we added a ConcentrationIndexer to calculate concentration, which also relies on TP, now I'm using the nested dict for conc and it works fine, so it's up to you whether to keep vol as the default, thanks!

yoelcortes commented 2 years ago

Hi Yalin,

I like it, but I would add one more tweak for speed:

    try:
        vol = self._data_cache['vol', TP]
    except:
        chemicals = self.chemicals
        mol = self.data
        vol = np.zeros_like(mol, dtype=object)
        for i, chem in enumerate(chemicals):
            vol[i] = VolumetricFlowProperty(chem.ID, mol, i, chem.V,
                                            TP, None, self._phase)
        self._data_cache['vol', TP] = vol = ChemicalVolumetricFlowIndexer.from_data(property_array(vol),
                                                      self._phase, chemicals,
                                                      False)

Feel free to make the change in thermosteam :)

Thanks!

yalinli2 commented 2 years ago

Smart! Updated here a62fc35b4057190234d1dcbeec3355ee99c6c670, thanks!