Skip to content

MAINT simplify compute_params_filterbank, remove _move_one_dyadic_step#848

Merged
MuawizChaudhary merged 3 commits intokymatio:devfrom
lostanlen:fix-847-bis
Jun 9, 2022
Merged

MAINT simplify compute_params_filterbank, remove _move_one_dyadic_step#848
MuawizChaudhary merged 3 commits intokymatio:devfrom
lostanlen:fix-847-bis

Conversation

@lostanlen
Copy link
Copy Markdown
Collaborator

@lostanlen lostanlen commented Jun 6, 2022

Fixes #847

lostanlen added 2 commits June 6, 2022 12:29
and remove _move_one_dyadic_step
(WIP, off-by-one error)
* the elbow_xi - q/Q * elbow_xi formula was wrong
* remove multiple assignments
* define js at the end of the function by means of a map on get_max_dyadic_subsampling (this third output will be removed from the prototype later)
@lostanlen lostanlen marked this pull request as ready for review June 6, 2022 21:50
@lostanlen lostanlen changed the title MAINT simplify compute_params_filterbank, remove and _move_one_dyadic_step MAINT simplify compute_params_filterbank, remove _move_one_dyadic_step Jun 6, 2022
@lostanlen lostanlen requested a review from MuawizChaudhary June 6, 2022 22:01
@lostanlen lostanlen added the 1D Issue with 1D scattering code label Jun 6, 2022
Copy link
Copy Markdown
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.

Comment thread kymatio/scattering1d/filter_bank.py Outdated
xis.append(elbow_xi - q/Q * elbow_xi)
sigmas.append(sigma_min)

js = list(map(get_max_dyadic_subsampling, xis, sigmas, repeat(alpha)))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i like this trick. I wonder if we could replace with a list comprehension tho.

Comment thread kymatio/scattering1d/filter_bank.py
@MuawizChaudhary MuawizChaudhary merged commit 8f6a773 into kymatio:dev Jun 9, 2022
@lostanlen
Copy link
Copy Markdown
Collaborator Author

thank you so much @MuawizChaudhary !!

@lostanlen lostanlen deleted the fix-847-bis branch June 25, 2022 16:25
eickenberg pushed a commit that referenced this pull request Jul 5, 2022
…step` (#848)

* simplify compute_params_filterbank

and remove _move_one_dyadic_step
(WIP, off-by-one error)

* bugfix previous commit

* the elbow_xi - q/Q * elbow_xi formula was wrong
* remove multiple assignments
* define js at the end of the function by means of a map on get_max_dyadic_subsampling (this third output will be removed from the prototype later)

* replace list-map by list comprehension

by request from Muawiz
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1D Issue with 1D scattering code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants