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

Fix successive comparisons in EXLA #523

Closed
wants to merge 1 commit into from
Closed

Conversation

seanmor5
Copy link
Collaborator

Successive comparisons fail in EXLA because they always return {:pred, 8} types and the compare operator in turn relies on Nx.Type.merge to merge result types in a successive comparison. Test is one example of something that fails before this. Doing something like:

y_true
|> Nx.equal(1)
|> Nx.as_type({:u, 8})
|> Nx.equal(2)

I'm pretty sure would workaround this, but I think this issue might manifest in other places as well, so I figured this was an easier workaround.

defp type_to_int(:bf), do: 3
defp type_to_int(:s), do: 2
defp type_to_int(:u), do: 1
defp type_to_int(:pred), do: 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we were calling this code, it would have been raising in the past, no?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this is what you mean, it was failing. Yeah, today Nx.Type doesn't know about pred. I would prefer if it still did not. I will investigate.

@josevalim josevalim closed this in e6980f1 Oct 20, 2021
@josevalim josevalim deleted the sm-fix-successive-compare branch October 20, 2021 19:54
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