pints
pints copied to clipboard
Ask-tell: Be explicit about allowed number of asks()
Currently the MCMC methods operate in one of two ways:
Form 1:
def ask():
if self._proposed is None:
self._proposed = ...
return self._proposed
def tell(fx):
if self._proposed is None:
raise RuntimeError('No proposal set / expecting ask')
...
self._proposed = None
In this method you can call ask() as often as you like between tells, it will just return the same proposal again and again and again. So ask tell ask tell is the same as ask ask ask tell ask ask tell ask ask. Two tells are never allowed ask tell tell = error.
Form 2:
def ask():
if self._ready_for_tell:
raise RuntimeError('Expecting tell()')
do_complex_stuff()
set_a_proposal()
self._ready_for_tell = True
def tell():
if not self._ready_for_tell:
raise RuntimeError('Expecting ask()')
self._ready_for_tell = False
In this method you're only allowed one ask() per tell. So ask ask tell is an error, as is ask tell tell.
So what
Form 2 is easier for complex methods. And we don't have a use case where form 1 is really necessary. So... should we explicitly disallow form 1?
@ben18785 @martinjrobins ?
Thoughts @martinjrobins ?
See also #994
Yes, I say disallow 1, ask is always followed by tell, which is always followed by ask. This should be documented in the base classes, but I don't think it matters wether individual methods allow you do do 1 though, as long as they support 2
Cool. @chonlei and @ben18785 do you agree?
If we go ahead with this, it would involve
- [ ] Update the documentation for the ask() method in the base classes for optimisation, mcmc, nested, and abc (and any others?)
- [ ] ~Update the tests for all methods. Some already have a check, I think, if not add in a check that a repeated ask raises the correct exception~
- [ ] ~Fix all methods that fail the tests~
- [ ] Check that no method has extra code to allow repeated asks. If allowed repeated asks follow naturally that's ok.
as I said, I don't think that methods that allow you do repeated asks should be flagged as needing to be fixed. I can't see the harm in allowing it, and its a lot more code to write and maintain in explicitly disallowing it
It could just give people the wrong impression, is what I'm thinking. They might write code that does repeated asks and then a while later change the method and report a 'bug'
agree that we still need the documentation in the ask/tell methods to make this clear
Alright, I can live with that