ivy
ivy copied to clipboard
Add kaiser_bessel_derived_window #17978 to tf_frontend_singal
Close https://github.com/unifyai/ivy/issues/17978
ivy-gardener :white_check_mark: Ivy gardener has formatted your code. If changes are requested, don't forget to pull your fork.
Frontend Task Checklist
IMPORTANT NOTICE 🚨:
The Ivy Docs represent the ground truth for the task descriptions and this checklist should only be used as a supplementary item to aid with the review process.
LEGEND 🗺:
- ❌ : Check item is not completed.
- ✅ : Check item is ready for review.
- 🆘 : Stuck/Doubting implementation (PR author should add comments explaining why).
- ⏩ : Check is not applicable to function (skip).
- 🆗 : Check item is implemented and does not require any edits.
CHECKS 📑:
-
- [x] 🆗: The function/method definition is not missing any of the original arguments.
-
- [x] ⏩: In case the function/method to be implemented is an alias of an existing function/method:
-
- [x] ⏩: It is being declared as such by setting
fun1 = fun2
, rather than being re-implemented from scratch.
- [x] ⏩: It is being declared as such by setting
-
- [x] ⏩: The alias is added to the existing function/method's test in the
aliases
parameter ofhandle_frontend_test
/handle_frontend_method
.
- [x] ⏩: The alias is added to the existing function/method's test in the
-
- [x] ⏩: In case the function/method to be implemented is an alias of an existing function/method:
-
- [x] 🆗: The naming of the function/method and its arguments exactly matches the original.
-
- [x] 🆗: No defined argument is being ignored in the function/method's implementation.
-
- [x] ⏩: In special cases where an argument's implementation should be pending due to an incomplete superset of an ivy function:
-
- [x] ⏩: A descriptive comment has been left under the
Implement superset behavior
ToDo list in https://github.com/unifyai/ivy/issues/6406.
- [x] ⏩: A descriptive comment has been left under the
-
- [x] ⏩: A ToDo comment has been added prompting to pass the frontend argument to the ivy function whose behavior is to be extended.
-
- [x] ⏩: In special cases where an argument's implementation should be pending due to an incomplete superset of an ivy function:
-
- [x] 🆗: In case a frontend function is being added:
-
- [x] 🆗: It is a composition of ivy functions.
-
- [x] ⏩: In case the needed composition is long (using numerous ivy functions), a
Missing Function Suggestion
issue has been opened to suggest a new ivy function should be added to shorten the frontend implementation.
- [x] ⏩: In case the needed composition is long (using numerous ivy functions), a
-
- [x] 🆗:
@to_ivy_arrays_and_back
has been added to the function.
- [x] 🆗:
-
- [x] 🆗: In case a frontend function is being added:
-
- [x] ⏩: In case a frontend method is being added:
-
- [x] ⏩: It is composed of existing frontend functions or methods.
-
- [x] ⏩: If a required frontend function has not yet been added, the method may be implemented as a composition of ivy functions, making sure that:
- [x] ⏩:
@to_ivy_arrays_and_back
has been added to the method. - [x] ⏩: A ToDo comment has been made prompting to remove the decorator and update the implementation as soon as the missing function has been added.
- [x] ⏩:
- [x] ⏩: If a required frontend function has not yet been added, the method may be implemented as a composition of ivy functions, making sure that:
-
- [x] ⏩: In case a frontend method is being added:
-
- [x] 🆗: The function/method's test has been added (except in the alias case mentioned in <2>):
-
- [ ] ❌: All supported arguments are being generated in
handle_frontend_test
/handle_frontend_method
and passed totest_frontend_function
/test_frontend_method
.
- [ ] ❌: All supported arguments are being generated in
-
- [ ] ❌: The argument generation covers all possible supported values. Array sizes, dimensions, and axes adhere to the full supported set of the original function/method.
-
- [ ] ❌: The
available_dtypes
parameter passed to the helper generating the function/method's input array is set tohelpers.get_dtypes("valid")
. If there are unsupported dtypes that cause the test to fail, they should be handled by adding@with_supported_dtypes
/@with_unsupported_dtype
to the function/method.
- [ ] ❌: The
-
- [x] 🆗: The function/method's test has been added (except in the alias case mentioned in <2>):
-
- [x] ❌: The PR is not introducing any test failures.
-
- [x] ✅: The lint checks are passing.
-
- [x] ❌: The implemented test is passing for all backends.
-
- [x] ❌: The PR is not introducing any test failures.
-
- [x] 🆗: The PR
closes
aSub Task
issue linked to one of the open frontend ToDo lists.
- [x] 🆗: The PR
-
- [x] 🆗: The function/method and its test have been added to the correct
.py
files corresponding to the addressed ToDo list.
- [x] 🆗: The function/method and its test have been added to the correct
-
- [x] 🆗: The PR only contains changes relevant to the addressed subtask.
If you are working on an open task, please edit the PR description to link to the issue you've created.
For more information, please check ToDo List Issues Guide.
Thank you :hugs:
ivy-gardener :white_check_mark: Ivy gardener has formatted your code. If changes are requested, don't forget to pull your fork.
Fixed the test case problems and while the rtol and atol arent needed I add them in signals related tests for tradition sake, quantization errors and such from an engineering perspective do demand such from time to time
Hey @Enigmafiend,
I've talked with @AnnaTz to make sure that I'm getting the details of the
dtype
issue right.
- Please don't add any changes unrelated to
kaiser_bessel_derived_window
.- You still need to add
dtype
to your test. It's required to test for every testable parameter.- You should generate
window_length
by usingdtype_and_values
withhelpers.get_dtypes("valid")
. You'll need to change the supported dtypes to integer ones.- Please don't add limits other than
min_value=0
beta
can be left being generated byhelpers.floats
but there is no need for limiting it
I havent made any changes unrelated to the current function, its just that for it to work for paddle, my kaiser_window fix will have to go through since the bessel derived window relies at its core on the pure kaiser window.
Secondly 3. You should generate window_length
by using dtype_and_values
with helpers.get_dtypes("valid")
. You'll need to change the supported dtypes to integer ones. will also result in tensors with ranks != 0 and hence will provide invalid inputs
Third > 4. Please don't add limits other than min_value=0
will cause the test to crash with extremely large inputs and as Ive mentioned in DMs and here aswell large betas above 80 will cause overflow aswell (Photo attached in the above thread)
I mean no offense but does the person you asked this about know why and how a kaiser window is used?
As for not limiting Beta whatsoever, I explained this extensively in the DMs but negative betas do not exist and the backends of these functions and the bessel function codes are written with those ideas in mind, the i0(x) is defined for only positive inputs so I see no point in reworking backends unnecessarily just to test for negative values of Beta. Also large betas cause overflow in all practical cases
As for Dtypes, I will request a re-review but before closing it for changes could you DM me as Ive asked a question about that to you in DMs aswell
As for Beta, apart from TF which normalizes overflow, other models go upto approx 88.7 for beta
after which they overflow as again betas above said value are not practically used or exist (I showed the math for this in DMs)
Hey, fixed an issue with Kaiser Window that would translate here aswell, as soon as that PR passes Ill update this one too
Hey, fixed an issue with Kaiser Window that would translate here aswell, as soon as that PR passes Ill update this one too
Cool, waiting for you