CalebBell / chemicals

chemicals: Chemical database of Chemical Engineering Design Library (ChEDL)
MIT License
186 stars 36 forks source link

Combustion #13

Closed yoelcortes closed 4 years ago

yoelcortes commented 4 years ago

The Hcombustion function right now does 2 things: find the stoichiometry, and calculate the higher heating value (HHV). Here are the enhancements I believe we could make:

In addition, we could make a lightweight CombustionData and CombustionStoichiometry object to store all the data generated to get HHV and leave LHV as a property (or not, the overhead almost unnoticeable ). For example:

from dataclasses import dataclass

def as_atoms(formula):
    if isinstance(formula, str):
        atoms = atoms_from_formula(formula)
    elif isinstance(formula, dict):
        atoms = formula
    else:
        raise ValueError("atoms must be either a string or dictionary, "
                        f"not a '{type(formula).__name__}' object")
    return atoms

@dataclass(frozen=True)
class CombustionStoichiometry:
    O2: float
    CO2: float
    Br2: float
    I2: float
    HCl: float
    HF: float
    SO2: float
    N2: float
    P4O10: float
    H2O: float
    Ash: float

    @classmethod
    def from_formula(cls, formula, MW=None):
        atoms = as_atoms(formula)
        ... # We would leave out the `combustion_stoichiometry` function I mentioned earlier
            # in favor of this class method

@dataclass(frozen=True)
class CombustionData:
    stoichiometry: CombustionStoichiometry
    HHV: float
    Hf: float
    MW: float

    @property
    def LHV(self):
        return LHV_from_HHV(HHV, self.stoichiometry.H2O)

    @classmethod
    def from_formula(cls, formula, Hf=None, MW=None, method=None):
        atoms = as_atoms(formula)
        if not MW:
            MW = molecular_weight_from_atoms(atoms)
        stoichiometry = CombustionStoichiometry.from_formula(atoms)
        if not method:
            method = 'Dulong' if Hf is None else 'Stoichiometry'
        if method == DULONG:
            HHV = HHV_modified_Dulong(stoichiometry, MW)
            if Hf: raise ValueError("cannot specify Hf if method is 'Dulong'")
            Hf = HHV - HHV_stoichiometric(stoichiometry, 0)
        elif method == STOICHIOMETRY:
            if Hf is None: raise ValueError("must specify Hf if method is 'Stoichiometry'")
            HHV = HHV_stoichiometric(stoichiometry, Hf)
        else:
            raise ValueError("method must be either 'Stoichiometric' or 'Dulong', "
                            f"not {repr(method)}")
        return cls(stoichiometry, HHV, Hf, MW)

Let me know what you think. I personally like how the data is frozen and there is no confusion with working with these classes, which are simply meant to hold data. Not sure if you've worked with dataclass, but there is always the asdict and astuple functions from the dataclasses library that make these easy to work with (although asdict and astuple are a little slow).

yoelcortes commented 4 years ago

@CalebBell, I added the combustion module plus the tests. I hope you're OK with using python's dataclass decorator, but we can always change this if not. It would be easy to change since the code is function oriented. I also added tests for each method and the scenarios I could think of, but I only directly test the combustion_data function. I plan on adding a few tests later for the lower level functions. I'll also add tests later for possible exceptions.

I added Perry's handbook as a reference (where I got the Dulong equation from) and made sure to add examples and units for each function. Let me know if I missed anything.

Also, I came across this warning:

tests\test_numba.py:162
  C:\Users\Yoel\OneDrive\Code\chemicals\tests\test_numba.py:162: PytestUnknownMarkWarning: Unknown pytest.mark.numba - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/latest/mark.html
    @pytest.mark.numba

I think you probably forgot to register the mark somewhere.

Thanks!

yoelcortes commented 4 years ago

I'll change the Dulong equation to not use molecular weight. Since it's used for coal, it doesn't make much sense to return hhv on a molar basis

yoelcortes commented 4 years ago

I'll remove the classes and just use dictionaries. I realized this is probably better on the long run

yoelcortes commented 4 years ago

This module is done! Tests and documentation are finished