Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add differentiation_method to PDELibrary and WeakPDELibrary #221

Closed
znicolaou opened this issue Jul 1, 2022 · 1 comment
Closed

Add differentiation_method to PDELibrary and WeakPDELibrary #221

znicolaou opened this issue Jul 1, 2022 · 1 comment
Assignees
Labels
enhancement New feature or request priority:low

Comments

@znicolaou
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

The current PDELibrary and WeakPDELibrary use FiniteDifference to calculate spatial derivatives, and take periodic and is_uniform options. This is an unnecessary limitation on the form of spatial derivatives use to calculate the library terms.

Describe the solution you'd like

We should add an optional differentiation_method argument to the library classes, along with an optional diff_kwargs to supply options like periodic and is_uniform as dictionary elements.

Additional context

This is a simple fix, but I am deferring now so that we can complete #185

@znicolaou znicolaou added enhancement New feature or request priority:low labels Jul 1, 2022
@znicolaou znicolaou self-assigned this Jul 1, 2022
@Jacob-Stevens-Haas
Copy link
Member

One thing that may shouldn't conflict but which comes close is #182. I have a branch implementing that where all differentation types store x as self.smoothed_x_. SmoothedFiniteDifference and SINDyDerivative actually do smooth x and save that instead. This design maintains backwards compatability of _differentiate, so it shouldn't conflict. feature_library.calc_trajectory accesses diff_method.smoothed_x_ to return both x_est, x_dot_est.

Thought they wouldn't conflict in code, there may be a conflict mathematically since model.fit could have already smoothed x before passing it to the feature library. If the feature library uses a differentiation method that also smooths x, it would end up applying the smoother twice.

jpcurbelo pushed a commit to jpcurbelo/pysindy_fork that referenced this issue May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority:low
Projects
None yet
Development

No branches or pull requests

2 participants