Skip to content
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

Added corrections to re-enable reciprocal test in math_brute_force suite for relaxed math mode #2221

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

shajder
Copy link
Contributor

@shajder shajder commented Jan 10, 2025

fixes #2145

As suggested by @svenvh reciprocal has different precision requirements than divide. This PR introduces special path for reciprocal for binar_float_operator to test reciprocal with relaxed math. If this PR will get approvals, invalidate PR #2162

@svenvh svenvh self-requested a review January 10, 2025 08:26
@bashbaug
Copy link
Contributor

Possibly dumb question: why are we introducing a special path through binary_operator_float to test reciprocal? Reciprocal is a unary operation, not a binary operation...

@shajder
Copy link
Contributor Author

shajder commented Jan 10, 2025

Possibly dumb question: why are we introducing a special path through binary_operator_float to test reciprocal? Reciprocal is a unary operation, not a binary operation...

Yep, good point. I went that way just because I wasn't able to choose a better spot. There is no unary float operator, right? To create one I would have to duplicate a lot of code. Any other clues?

@svenvh
Copy link
Member

svenvh commented Jan 13, 2025

Possibly dumb question: why are we introducing a special path through binary_operator_float to test reciprocal? Reciprocal is a unary operation, not a binary operation...

Yep, good point. I went that way just because I wasn't able to choose a better spot. There is no unary float operator, right? To create one I would have to duplicate a lot of code. Any other clues?

Treating reciprocal as a binary operator with its first argument being the constant 1 seems the least worst solution for now. I'm in the process of reducing code duplication in the math bruteforce suite, so eventually we may be able to move it into its own place. But for now I'm okay with @shajder's chosen solution.

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.

Re-enable reciprocal test in math_brute_force suite
3 participants