Fix ScalarLoop until behavior when n_steps <= 0#1936
Fix ScalarLoop until behavior when n_steps <= 0#1936vkverma9534 wants to merge 1 commit intopymc-devs:v3from
Conversation
|
Actually I think we should do done=False, we never checked if the condition become true. I think the C backend and PyTorch are actually doing the opposite, and those should be changed? Either way the docstrings should agree with the implementation, and we should have test to ensure the same behavior across backends |
|
@ricardoV94 That makes sense. My change followed the current docstring, which says the loop is skipped and done=True when n_steps <= 0. But I agree it could also be interpreted as done=False since the until condition was never evaluated. I think the key point is making sure the Python, C, and PyTorch backends behave consistently. I'm happy to adjust the implementation or the docstring depending on the preferred behavior. |
|
Also if you have time I would appreciate your review on pymc-devs/pymc-examples#848 it is already discussed in this issue pymc-devs/pymc#8112 |
We're a bit swamped these days, so expect some delay in feedback |
2 participants
ScalarLoop.perform returns done=False when n_steps <= 0 for loops with an until condition, even though the docstring specifies that the loop should be skipped and done=True in this case. this PR fixes the implementation to match the documented behavior and adds a small regression test. I also added a few inline comments for clarity, but the change itself is quite straightforward; Im happy to remove the comments if preferred.