FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Remove assert that check that signal/filter types have to be the same by umar456 · Pull Request #2993 · arrayfire/arrayfire · GitHub

Repository navigation

Remove assert that check that signal/filter types have to be the same - #2993

Merged
9prady9 merged 1 commit into
arrayfire:masterfrom
umar456:rm_conv_assert
Aug 14, 2020
Merged

9prady9 merged 1 commit into
arrayfire:masterfrom
umar456:rm_conv_assert

Conversation

umar456 commented Aug 13, 2020

Copy link
Copy Markdown
Member

Remove the check that ensures that the filter type and signal type have to be the same.

Description

The filter and signal type need to be the same to function properly but ArrayFire used to perform implicit conversions if the filter type does not match the signal type. A check was added in the previous release that added an assert to enforce the same type which broke some code. This PR removes that check.

This sort of thing should warn the user in case this is being performed. We can also add a flag that does perform these checks for users who are concerned about these types of conversions. Although the performance impact of performing these types of conversions are minimal.

Changes to Users

Users do not explicitly have to convert the filter to the signal type before calling fftconvolve

Checklist

  • Rebased on latest master
  • Code compiles
  • Tests pass
  • [ ] Functions added to unified API
  • [ ] Functions documented

umar456 added this to the 3.8.0 milestone Aug 13, 2020
9prady9 merged commit d849785 into arrayfire:master Aug 14, 2020
9prady9 deleted the rm_conv_assert branch August 14, 2020 05:25
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL