[BUGFIX] grad_2f1: Add minimum number of iterations and early stopping test

[andrjohns]

Summary

As mentioned in #2857, the grad_2f1 function can falsely converge early when only calculating gradients w.r.t. a subset of parameters. This PR introduces a minimum number of iterations (5) to avoid this and a test to verify

Tests

Test-case from #2857 added

Side Effects

N/A

Release notes

Added a minimum number of iterations (5) to the grad_2f1 function to avoid early convergence

Checklist

• Math issue grad2F1 can terminate too early when only some gradients are requested #2857

• Copyright holder: Andrew Johnson

  The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
  - Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
  - Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

• the basic tests are passing

  • unit tests pass (to run, use: ./runTests.py test/unit)
  • header checks pass, (make test-headers)
  • dependencies checks pass, (make test-math-dependencies)
  • docs build, (make doxygen)
  • code passes the built in C++ standards checks (make cpplint)

• the code is written in idiomatic C++ and changes are documented in the doxygen

• the new changes are tested

[andrjohns]

@spinkney would you be able to have a look at this? Do you think 5 iterations is enough or should we go for 10?

[ricardoV94]

@spinkney would you be able to have a look at this? Do you think 5 iterations is enough or should we go for 10?

I used 10 when adapting your code here pymc-devs/pytensor#90

I am using the same test cases you guys had, but the algorithm always computes one partial derivative at a time. With 5 minimum iterations the tests wouldn't all pass, but I am not sure I am asserting the same precision as the STAN tests.

It feels pretty arbitrary regardless :)

[syclik]

@andrjohns, just asking for a change for readability. I don't think it's worth adding it to the signature.

I think it'd be ok bumping it up to 10 if you want. Or leaving it at 5. It is pretty arbitrary at this point.

[spinkney]

@spinkney would you be able to have a look at this? Do you think 5 iterations is enough or should we go for 10?

I think 10 is better. Is there any way you can do a grid search across possible values to see if there is a problematic range where even 10 iterations is not enough? If you can't find any ranges that's fine. If you do find a problematic range then we can document it and see what else we can do (more iterations or notes to future devs to improve the algorithm in that range).