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

Remove vectorize + write frontend-agnostic runtime checks #861

Merged
merged 4 commits into from
Jun 9, 2022

Conversation

lostanlen
Copy link
Collaborator

@lostanlen lostanlen commented Jun 9, 2022

Fix #843
(a second attempt after some productive discussions during review of #844)

This is a planned change for v0.3

The default behavior is still out_type="array" (vectorize=True)
We introduce out_type="dict" to replicate the behavior of out_type="array", vectorize=False

A net reduction in code size thanks to utility methods _check_runtime_args() and _check_input() in ScatteringBase1D

Written on top of #860. I suggest we look at #860 first
Tests pass locally

Note that vectorize is still documented in the base frontend. I'll take care of updating these docs after code review.

Copy link
Collaborator

@MuawizChaudhary MuawizChaudhary left a comment

Choose a reason for hiding this comment

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

Pro merging once the Jenkins finishes building

@MuawizChaudhary
Copy link
Collaborator

hmmmm seems like the Jenkins aborted

@lostanlen
Copy link
Collaborator Author

but Jenkins passed on #862, which contains this PR ?!

@MuawizChaudhary
Copy link
Collaborator

Tests pass on my end, dunno whats up with Jenkins here.

@MuawizChaudhary MuawizChaudhary merged commit 4a6d9ea into kymatio:dev Jun 9, 2022
@lostanlen lostanlen deleted the fix-843-bis branch June 25, 2022 16:24
eickenberg pushed a commit that referenced this pull request Jul 5, 2022
* avoid passing log2_T, pad, unpad to 1D core

* remove vectorize from Scattering1D

use out_array='dict' instead !

* finish removing vectorize

Co-authored-by: Muawiz Sajjad Chaudhary <39755015+MuawizChaudhary@users.noreply.github.com>
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.

2 participants