ContextCache retrieves same context with two consecutive calls. #252
Conversation
| # Multiple matches, prefer the non-default Integrator. | ||
| # Always pick the least recently used to make two consective | ||
| # calls retrieving the same integrator. | ||
| for context_id in reversed(matching_context_ids): | ||
| if context_id[1] != self._default_integrator_id(): | ||
| context = self._lru[context_id] | ||
| break |
There was a problem hiding this comment.
Somewhat related to the problem, but actually older
Although you make this "prefer" the non default integrator, won't this never select the default integrator? Since you reject every integrator id matching default, I don't see where the default selection actually comes in. Wouldn't you need a line before the for loop starts to select the default integrator and replace it in the loop if the if condition is met?
There was a problem hiding this comment.
Sorry for the confusion, the default integrator should actually be selected only if it's the only available option in the elif just above it.
There was a problem hiding this comment.
Okay, so let me run through the list of conditions to make sure its all handled I don't miss anything:
- No context: pick the default (main
if) - One context (could be the default), pick the one (
elif) - Multiple contexts, may be default, try to pick the non-default (
else)
Is there ever going to be a case where multiple contexts are picked and they are all the default integrator? I know that each context/integrator pair is unique, but the default can just be cloned and fed into multiple contexts. I don't know if by this point the default is paired up and can't be used again.
There was a problem hiding this comment.
Exactly.
but the default can just be cloned and fed into multiple contexts
If it happens, it means that the thermodynamic states of the multiple contexts are incompatible, so only one will end up in matching_context_ids.
There was a problem hiding this comment.
Okay, so long as its accounted for. Switching to approve
This fixes a problem where in some cases
ContextCache.get_context(state, integrator=None)would pick two differentContexts after two consecutive calls with the samestate. The advantage of having always the same context is that the user doesn't necessarily have to set positions and save some time.