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

Differentiation of Multidimensional Arrays in SINDyDerivative #476

Merged
merged 11 commits into from
Jul 15, 2024

Conversation

yb6599
Copy link
Collaborator

@yb6599 yb6599 commented Feb 21, 2024

This PR addresses the issue and lets SINDyDerivative handle arrays with more dimensions than 2.

Copy link

codecov bot commented Feb 21, 2024

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 94.42%. Comparing base (024bc9e) to head (f4b8be7).
Report is 11 commits behind head on master.

Current head f4b8be7 differs from pull request most recent head 30188e9

Please upload reports for the commit 30188e9 to get more accurate results.

Files Patch % Lines
pysindy/differentiation/sindy_derivative.py 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
- Coverage   94.68%   94.42%   -0.27%     
==========================================
  Files          38       38              
  Lines        4105     4069      -36     
==========================================
- Hits         3887     3842      -45     
- Misses        218      227       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@Jacob-Stevens-Haas Jacob-Stevens-Haas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, thanks Yash! Don't worry about code coverage, this looks like it passes CI. I just want to make the test a little bit more robust.

test/test_differentiation.py Outdated Show resolved Hide resolved
@Jacob-Stevens-Haas Jacob-Stevens-Haas linked an issue Feb 29, 2024 that may be closed by this pull request
Copy link
Member

@Jacob-Stevens-Haas Jacob-Stevens-Haas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Yash, thanks for making work on this. A few tips to make it work like a proper test

test/test_differentiation.py Outdated Show resolved Hide resolved
test/test_differentiation.py Outdated Show resolved Hide resolved
test/test_differentiation.py Outdated Show resolved Hide resolved
@Jacob-Stevens-Haas
Copy link
Member

LGTM, but we'll wait to merge it until the derivative PR, since we'll probably have to set the requirements to 0.6.2

Copy link
Member

@Jacob-Stevens-Haas Jacob-Stevens-Haas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks Yash!

@Jacob-Stevens-Haas Jacob-Stevens-Haas merged commit 15e8093 into dynamicslab:master Jul 15, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] SINDyDerivative does not handle PDEs
2 participants