Add bisection and brent's method for root finding #424
Conversation
I added two jitted robust root finding methods: Bisection and Brentq which are based on Scipy's version which is written in C. They both follow the previous jitted root finding procedures by returning a namedtuple with relevant information. I also added a basic test for each method.
|
Thanks! Nice code and nicely documented. This looks good to me. |
quantecon/optimize/__init__.py
Outdated
| @@ -3,4 +3,4 @@ | |||
| """ | |||
|
|
|||
| from .scalar_maximization import brent_max | |||
| from .root_finding import newton, newton_halley, newton_secant | |||
| from .root_finding import * | |||
There was a problem hiding this comment.
hi @spvdchachan I would rather list the items here rather than using *. That way if someone adds a support function that isn't prepended with _ it won't get promoted through import.
There was a problem hiding this comment.
actually I just realised you have used __all__ in root_finding.py. Please ignore.
|
@mmcky I've fixed |
There was a problem hiding this comment.
This looks nice (I did not go into details though). Minor comments:
- Default values for optional arguments should be described in the docstrings.
- There are several "trailing whitespaces". Check with http://pep8online.com for example.
quantecon/optimize/root_finding.py
Outdated
| @@ -2,26 +2,31 @@ | |||
| from numba import jit, njit | |||
There was a problem hiding this comment.
jit seems not to be used in this file.
|
thanks @spvdchachan this is looking great. I agree with @oyamad it would be nice to add the |
I have included optional default arguments in the docstring of all functions. All trailing whitespaces are now fixed conforming to PEP8. The check is done via pycodestyle and http://pep8online.com/.
|
@oyamad @mmcky I agree with adding default values to the documentation. Hence, I've added them to all functions (including other root finding procedures to be consistent). I also fixed issues with trailing whitespaces to follow pep8 (checked through With regards to using variables inside docstrings, one option is to use a decorator as discussed in https://stackoverflow.com/questions/10307696/how-to-put-a-variable-into-python-docstring. |
|
thanks for the updates @spvdchachan. I am happy with values for now. that link is interesting - I think the decorator approach would be the best but there is not need to implement this before merging. I'll think a bit more about if we want to pursue filling the docstrings from variables for default values. |
Hi @jstac and @mmcky
I would like to resolve #422 in this PR.
I've added two new robust root finding methods
bisectandbrentqwhich are based on Scipy's version.These procedures were written in C originally and I rewrote them in Python to jit compile.