brainpy icon indicating copy to clipboard operation
brainpy copied to clipboard

Simple formulas like H2O or HPO4 crash parse_formula()

Open dcf1007 opened this issue 5 years ago • 3 comments

When you have a formula with only 1 atom of the element (let's say H2O or HPO4), the function "parse_formula(formula)" would crash in line 123: composition[_make_isotope_string(elem, int(isotope) if isotope else 0)] += int(number):

ValueError: invalid literal for int() with base 10: ''

Changing the line to: composition[_make_isotope_string(elem, int(isotope) if isotope else 0)] += int(number) if number else 1 fixes that error.

I have requested a pull request to fix the bug #1

dcf1007 avatar Dec 14 '19 11:12 dcf1007

Thank you for taking the time to review the problem. Did you run this with the C-extensions enabled?

The C formula parser does not handle implicit single atoms either, but is a hand-written state machine based parser for efficiency, where this behavior would be more difficult to achieve. I left this behavior off of the Python implementation to maintain parity between the two versions.

mobiusklein avatar Dec 14 '19 17:12 mobiusklein

To follow up on this one, is there any documentation on how to use the C extensions from python code?

sanderrouk avatar May 25 '22 13:05 sanderrouk

If the C extensions are compiled, they are used automatically.

In [1]: import brainpy
In [2]: brainpy.isotopic_variants
Out[2]: <function brainpy._c.isotopic_distribution.pyisotopic_variants>
In [3]: brainpy.parse_formula
Out[3]: <function brainpy._c.composition.parse_formula>

If you try to use the object-oriented API:

In [4]: brainpy.IsotopicDistribution
Out[4]: brainpy._speedup.IsotopicDistribution

Otherwise, these names fall back to the pure Python implementation which is the same algorithm, albeit slower.

mobiusklein avatar May 25 '22 14:05 mobiusklein