search

mobiusklein / brainpy

A Python implementation of Baffling Recursive Algorithm for Isotopic distributioN calculations
http://mobiusklein.github.io/brainpy
Apache License 2.0
19 stars 11 forks source link

Simple formulas like H2O or HPO4 crash parse_formula() #2

Open dcf1007 opened 4 years ago

dcf1007 commented 4 years ago

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

mobiusklein commented 4 years ago

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.

sanderrouk commented 2 years ago

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

mobiusklein commented 2 years ago

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.