-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Register signal ops for op set 17. #11733
Conversation
Also: * de-duplicate get_scalar_value_from_tensor * fix some bugs that caused compilation errors with the experimental ops. Tested with `build.sh --ms_experimental` [ghstack-poisoned]
Also: * de-duplicate get_scalar_value_from_tensor * fix some bugs that caused compilation errors with the experimental ops. Tested with `build.sh --ms_experimental` ghstack-source-id: d782602713cfc06f495d1a44dcddaf38aa965e71 Pull Request resolved: #11733
Also: * de-duplicate get_scalar_value_from_tensor * fix some bugs that caused compilation errors with the experimental ops. Tested with `build.sh --ms_experimental` [ghstack-poisoned]
Also: * de-duplicate get_scalar_value_from_tensor * fix some bugs that caused compilation errors with the experimental ops. Tested with `build.sh --ms_experimental` * fix some spelling errors ghstack-source-id: 1e0f8206a848eaf52b381ac78cb8d35f4e8da5e0 Pull Request resolved: #11733
Also: * de-duplicate get_scalar_value_from_tensor * fix some bugs that caused compilation errors with the experimental ops. Tested with `build.sh --ms_experimental` [ghstack-poisoned]
Also: * de-duplicate get_scalar_value_from_tensor * fix some bugs that caused compilation errors with the experimental ops. Tested with `build.sh --ms_experimental` * fix some spelling errors ghstack-source-id: 729a2230adbed583c044653b25dbce8c54a29e30 Pull Request resolved: #11733
@smk2007 we have a special build arg, "--ms_experimental", for enabling the audio ops. Does it mean that after this change we can remove the build arg from build.py? |
This pull request introduces 5 alerts and fixes 6 when merging 74dffcd into 0b7557f - view on LGTM.com new alerts:
fixed alerts:
|
I think we should keep it for when microsoft needs to make experimental contrib op additions that are specific to the winml package. After this change i think it should persist until we are certain that we can deprecate, and customers have moved to onnx 1.12 audio ops. In reply to: 1146453095 |
Or is the intention to keep them as functions? Seems like these implementations would be faster.. In reply to: 1146478057 Refers to: onnxruntime/contrib_ops/cpu/signal/window_functions.h:26 in 74dffcd. [](commit_id = 74dffcd, deletion_comment = False) |
Then forever we would have the needs that someone wants an op that is in WindowsAI package but not in the macOS packages we provide. When the customer comes to us, then what should we do? |
I believe that this namespace will be useful for platform specific (and or dependent) efforts, and will allow us to react to windows customers quickly. For cases where operators should truly be x-plat, i believe we would then begin the process to upstream them to onnx like we did with these audio ops. |
As they are not directly related to this PR, I will ping you next week and discuss it with you offline. |
IIRC we dont want to add contrib ops widely, so wouldnt x-plat folks have to go through that pain anyway right? In reply to: 1146482654 |
Also: * de-duplicate get_scalar_value_from_tensor * fix some bugs that caused compilation errors with the experimental ops. Tested with `build.sh --ms_experimental` * add function bodies for ms experimental ops * fix some spelling errors ghstack-source-id: 729a2230adbed583c044653b25dbce8c54a29e30 Pull Request resolved: microsoft#11733
Also: * de-duplicate get_scalar_value_from_tensor * fix some bugs that caused compilation errors with the experimental ops. Tested with `build.sh --ms_experimental` * add function bodies for ms experimental ops * fix some spelling errors ghstack-source-id: 729a2230adbed583c044653b25dbce8c54a29e30 Pull Request resolved: microsoft#11733
Switching to #11778, sorry for the noise. |
Stack from ghstack:
Also:
ops. Tested with
build.sh --ms_experimental