Conversation
pavelkomarov
commented
Aug 11, 2025
| """This module implements some common finite difference schemes | ||
| """ | ||
| from ._finite_difference import first_order, second_order, fourth_order | ||
| from ._finite_difference import finite_difference, first_order, second_order, fourth_order |
Collaborator
Author
There was a problem hiding this comment.
There are simply more public methods now. The docstring makes reference to the fact other methods in the module really just call the backing one.
pavelkomarov
commented
Aug 11, 2025
| def _finite_difference(x, dt, num_iterations, order): | ||
| """Helper for all finite difference methods, since their iteration structure is all the same. | ||
| def finite_difference(x, dt, num_iterations, order): | ||
| """Perform iterated finite difference of a given order. This serves as the common backing function for |
Collaborator
Author
There was a problem hiding this comment.
Had to add more proper docstrings.
…rward and backward difference, it doesn't do as well as just plain first order at the edges
pavelkomarov
commented
Aug 11, 2025
| """ | ||
| xhat0 = np.zeros(A.shape[0]); xhat0[0] = x[0] # See #110 for why this choice of xhat0 | ||
| q = 10**int(np.log10(qr_ratio)/2) # even-ish split of the powers across 0 | ||
| r = q/qr_ratio |
…referred order rather than alphabetically
This was referenced Aug 12, 2025
pavelkomarov
commented
Aug 12, 2025
| """ | ||
| from ._kalman_smooth import constant_velocity, constant_acceleration, constant_jerk, known_dynamics | ||
|
|
||
| __all__ = ['constant_velocity', 'constant_acceleration', 'constant_jerk', 'known_dynamics'] # So these get treated as direct members of the module by sphinx |
Collaborator
Author
There was a problem hiding this comment.
For modules where I manually reference methods in the .rst, there is no need to list out __all__ anymore.
pavelkomarov
commented
Aug 12, 2025
| method_params_and_bounds = { | ||
| spectraldiff: ({'even_extension': [True, False], | ||
| 'pad_to_zero_dxdt': [True, False], | ||
| spectraldiff: ({'even_extension': (True, False), # give boolean or numerical params in a list to scipy.optimize over them |
pavelkomarov
commented
Aug 12, 2025
| :param np.array[float] dxdt_truth: actual time series of the derivative of x, if known | ||
| :param float tvgamma: Only used if :code:`dxdt_truth` is given. Regularization value used to select for parameters | ||
| that yield a smooth derivative. Larger value results in a smoother derivative. | ||
| :param dict search_space_updates: At the top of :code:`_optimize.py`, each method has a search space of parameters |
Collaborator
Author
There was a problem hiding this comment.
I renamed this param, because the old name was a little too grandiose for what it is.
pavelkomarov
commented
Aug 12, 2025
|
|
||
| methods = [second_order, fourth_order, mediandiff, meandiff, gaussiandiff, friedrichsdiff, butterdiff, | ||
| splinediff, spectraldiff, polydiff, savgoldiff, constant_velocity, constant_acceleration, constant_jerk] | ||
| methods = [finite_difference, mediandiff, meandiff, gaussiandiff, friedrichsdiff, butterdiff, |
Collaborator
Author
There was a problem hiding this comment.
shorter list now
pavelkomarov
commented
Aug 12, 2025
|
|
||
| def metrics(x, dt, x_hat, dxdt_hat, x_truth=None, dxdt_truth=None, padding=0): | ||
| """Evaluate x_hat based on various metrics, depending on whether dxdt_truth and x_truth are known or not. | ||
| def rmse(x, dt, x_hat, dxdt_hat, x_truth=None, dxdt_truth=None, padding=0): |
Collaborator
Author
There was a problem hiding this comment.
I decided to rename this one, because all the metrics it's calculating are RMSE-related.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I decided to take on a bit of #138 with a "yes and". The old methods all live where they lived, but I've exposed the common backing methods of TVR, Kalman, and Iterated FD. In the process of the RTS smoothing/Kalman one, I took on #139, because I couldn't help myself.
Of course that then entailed some changes to the tests and to the optimization code. Most profound, I realized Iterated FD has numerical order, but only some are implemented, so the search space of that parameter needs to be categorical. Handling categoricals is a challenge I chose to leave alone last time I worked on the optimizer, but happy to say I've successfully wrestled that bear.