Fix #481: add retry logic to pubchem fetching function #1113
Conversation
I added a retry mechanism to the function `geometry_from_pubchem()` in the file `src/openfermion/chem/pubchem.py`. It now catches exceptions and reries network call up to three times with a five-second delay between each attempt. I also updated the corresponding tests in `src/openfermion/chem/pubchem_test.py` to mock the network calls and verify the new retry logic.
mhucka
added
announcement
When adding a new term to a `SymbolicOperator` with a sympy coefficient, the coefficient was being cast to a float. This was because the default value for a new term was `0.0`, which is a float. This commit changes the default value to `0`, which is an integer. This ensures that the type of the coefficient is preserved when adding new terms.
This should address the code coverage failure in the CI checks.
There was a problem hiding this comment.
Please do not add hard-coded retries to geometry_from_pubchem.
We cannot presume users would always want to retry the pubchempy.get_compounds call.
They may want it to fail quickly and handle the failure in their own way. As it is, they'd be getting an extra 10 second delay before the function finally throws.
If the goal here is to avoid server related flakes in pubchem_test, a better option is to define a mock pubchempy.get_compounds function, which returns hard-coded results for the molecules used in the test. The tests can then run with mocked get_compounds.
If you'd like to also have a test with an actual pubchempy.get_compounds call, the place to add retries is in that test rather than in geometry_from_pubchem.
mhucka
force-pushed
the
master
branch
2 times, most recently
from
68c920e to
5fcc1d8
Compare
The nightly test runs would often fail at least one test that fetched data from a Pubchem network service. The failures happened because sometimes the remote server would respond with an error (probably because it's getting hit with too many requests). The problem was reported years ago in issue #481.
This PR should address this problem. I added a simple retry mechanism to the function
geometry_from_pubchem()in the filesrc/openfermion/chem/pubchem.py. It now catches exceptions and retries the network call up to three times with a delay between each attempt. I also updated the corresponding tests insrc/openfermion/chem/pubchem_test.pyto mock the network calls and verify the new retry logic.