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.

Sign up for GitHub

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

Jump to bottom

Add filtfilt to cupyx.scipy.signal #7496

Merged
merged 10 commits into from
May 16, 2023
Merged

Add filtfilt to cupyx.scipy.signal #7496

merged 10 commits into from
May 16, 2023

Conversation

andfoy
Copy link
Contributor

@andfoy andfoy commented Mar 29, 2023

See #7403

This is basically a line-per-line port from SciPy, the only changes are related to the initial state differences between CuPy and SciPy's lfilter implementation

@andfoy andfoy mentioned this pull request Mar 29, 2023
Open
@takagi takagi self-assigned this Mar 30, 2023
@takagi takagi added cat:feature New features/APIs prio:medium labels Mar 30, 2023
@andfoy andfoy force-pushed the add_filtfilt branch 3 times, most recently from 1a8fb5d to d4aeb41 Compare April 3, 2023 23:01
@mergify
Copy link
Contributor

mergify bot commented Apr 10, 2023

This pull request is now in conflicts. Could you fix it @andfoy? 🙏

@mergify
Copy link
Contributor

mergify bot commented Apr 11, 2023

This pull request is now in conflicts. Could you fix it @andfoy? 🙏

@mergify
Copy link
Contributor

mergify bot commented Apr 13, 2023

This pull request is now in conflicts. Could you fix it @andfoy? 🙏

@andfoy andfoy marked this pull request as ready for review April 13, 2023 16:35
@takagi
Copy link
Member

takagi commented Apr 17, 2023

/test mini

@takagi takagi added this to the v13.0.0a1 milestone Apr 17, 2023
@takagi
Copy link
Member

takagi commented Apr 17, 2023 •
edited

Seems that CI makes illegal memory access and fails on CUDA 10.2. I guess some dtype combinations make this problem. Would you try to simply let dtypes parametrized instead of for_all_dtypes_combination?

@mergify
Copy link
Contributor

mergify bot commented Apr 17, 2023

This pull request is now in conflicts. Could you fix it @andfoy? 🙏

@takagi
Copy link
Member

takagi commented Apr 20, 2023

/test mini

@takagi
Copy link
Member

takagi commented Apr 20, 2023

/test mini

@takagi
Copy link
Member

takagi commented Apr 27, 2023

/test mini

@takagi
Copy link
Member

takagi commented May 8, 2023

/test mini

@andfoy
Copy link
Contributor Author

andfoy commented May 9, 2023

@takagi, the errors are related to some Cython compilation error, do you know if this is a failure on the CIs? The tests are passing just fine in other CUDA versions, as well as my local installation

@takagi
Copy link
Member

takagi commented May 16, 2023

Yes, let us merge this

@takagi takagi enabled auto-merge May 16, 2023 04:12
@takagi
Copy link
Member

takagi commented May 16, 2023

Thanks, @andfoy!

@takagi takagi disabled auto-merge May 16, 2023 04:14
@takagi takagi merged commit c5a81b9 into cupy:main May 16, 2023
47 of 50 checks passed
@andfoy andfoy deleted the add_filtfilt branch May 16, 2023 08:14
@andfoy andfoy mentioned this pull request May 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@takagi takagi takagi left review comments

Assignees

@takagi takagi

Labels
cat:feature New features/APIs prio:medium
Projects
Signal API (CZI EOSS 5)
Status: ✅ Done
Milestone
v13.0.0a1
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@andfoy @takagi