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

Add dtype argument to define_constant #2423

Merged
merged 6 commits into from
Feb 9, 2023
Merged

Add dtype argument to define_constant #2423

merged 6 commits into from
Feb 9, 2023

Conversation

jacobhinkle
Copy link
Collaborator

This PR adds a dtype argument to define_constant. Internally, the data is held in the larger type: e.g. using define_constant(3.5, dtype=DataType.Float) does not cast 3.5 to float, and does not affect the literal that is placed in the generated kernel. However, it does affect the type promotion rules for ops that determine the DataTypes of outputs. The added test demonstrates this behavior in where().

Fixes #2380

This test ensures that given two Float constants, where infers the
output dtype properly.
@jacobhinkle
Copy link
Collaborator Author

Note that the complex examples in the test are commented as I am currently getting a TypeError with those. I'd like to wait to merge until understanding that issue.

The complex constant tests currently fail with a TypeError.
This fixes the issues I was having with defining complex constants. The
problem was that the Python frontend actually generates Python code
which gets evaluated, and that Python code was simply printing the
values assuming they were real (if not bool). In the case of
std::complex, C++ prints those as pairs i.e. a+b*j is printed as (a, b),
which in Python is a tuple, not a complex value. Instead, we now print
them as a+bj which is the compact form for a Python complex literal.
@jacobhinkle
Copy link
Collaborator Author

Fixed complex printing in ConstantRecord with 36c6c5b, so now I believe all dtypes are working. Tests are passing.

@jacobhinkle jacobhinkle marked this pull request as ready for review February 8, 2023 17:45
Copy link
Collaborator

@kevinstephano kevinstephano left a comment

Choose a reason for hiding this comment

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

LGTM!

@jacobhinkle jacobhinkle merged commit c411f97 into devel Feb 9, 2023
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.

Python where op floatxfloat promotes to float64
2 participants