Openff-toolkit: Remove one between ToolkitWrapper.toolkit_is_available and ToolkitWrapper.is_available

[andrrizzi]

I'd vote for removing toolkit_is_available() so that the final syntax would become something like

toolkit = RDKitToolkitWrapper()
toolkit.is_available()

instead of toolkit.toolkit_is_available().

[j-wags]

Fully agree.

[jchodera]

In my initial implementation, I distinguished between the two:

• ToolkitWrapper.toolkit_is_available() is a static method that always does whatever work is needed to check if the toolkit is really available. It can be called at any time.
• ToolkitWrapper.is_available() is a class method that caches the result of the (possibly expensive) check from ToolkitWrapper.toolkit_is_available() inside a class field, but is only useful if the class exists. Once the cls._is_available field has been set, it will not check again, which could lead to trouble in certain instances.

[andrrizzi]

I see. Thanks for clarifying, I missed that bit. Let me do some timings to check the advantages.

[andrrizzi]

These are the results

import timeit

number = 1000

from openforcefield.utils.toolkits import OpenEyeToolkitWrapper
time = timeit.timeit(
    stmt='OpenEyeToolkitWrapper.toolkit_is_available()',
    number=1000,
    globals=globals()
)
print('toolkit_is_available():', time/number)

toolkit = OpenEyeToolkitWrapper()
time = timeit.timeit(
    stmt='toolkit.is_available()',
    number=1000,
    globals=globals()
)
print('is_available():        ', time/number)

and I got

toolkit_is_available(): 0.00013428298599319532
is_available():         1.8775599892251194e-07

The cached implementation is 3 orders of magnitude faster, but I'm tempted to remove it anyway to simplify the API since the method is so cheap compared to the actual parametrization step anyway.

[jchodera]

The cached implementation is 3 orders of magnitude faster, but I'm tempted to remove it anyway to simplify the API since the method is so cheap compared to the actual parametrization step anyway.

Sounds good! At the time I wrote it, I was expecting license checks to take a significant fraction of a second to a few seconds, and was worried about the case where we were parameterizing many molecules. But we can optimize later after profiling!

[andrrizzi]

Sounds good!

[j-wags]

Good idea to check the timing on this, @andrrizzi. I agree that we should remove the cached implementation.