-
Notifications
You must be signed in to change notification settings - Fork 124
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
Signal #580
Signal #580
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic! Can you add owl_signal.mli
with the interface to expose and the corresponding documentation?
Great work! As @mseri commented, it would be great to have an mli file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! thanks for the PR.
Please do verify the updates. We have altered the tests and added an MLI file. Happy to make changes as needed. Thanks. |
Looks very good to me. @mseri comments? |
Looks great to me. I would suggest extending this module to both single and double precision, but that can be revised in a future PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for the changes.
I fully agree with @jzstark
This is our initial attempt to address issue #564.