Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dcf1007 opened this issue Dec 14, 2019 · 3 comments
Open

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

dcf1007 opened this issue Dec 14, 2019 · 3 comments

Comments

@dcf1007
Copy link

dcf1007 commented Dec 14, 2019

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
Copy link
Owner

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
Copy link

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

@mobiusklein
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants