Skip to content

TypeInferenceMapper: allow np.bool in map_type_cast #856

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.

Sign up for GitHub

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

Jump to bottom
Merged
inducer merged 1 commit into from
Jul 19, 2024
Merged

TypeInferenceMapper: allow np.bool in map_type_cast #856

inducer merged 1 commit into from
Jul 19, 2024

Conversation

@matthiasdiener
Copy link
Collaborator

@matthiasdiener matthiasdiener commented Jul 18, 2024 •
edited
Loading

9f2e8ce broke some mirgecom runs:

[...]
  File "/home/runner/work/mirgecom/mirgecom/src/loopy/loopy/type_inference.py", line 329, in map_sum
    dtype_set = self.rec(child)
                ^^^^^^^^^^^^^^^
  File "/home/runner/work/mirgecom/mirgecom/src/pymbolic/pymbolic/mapper/__init__.py", line 271, in __call__
    result = method(expr, *args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/mirgecom/mirgecom/src/loopy/loopy/type_inference.py", line 400, in map_type_cast
    raise LoopyError(f"Can't cast a '{subtype}' to '{expr.type}'")
loopy.diagnostic.LoopyError: Can't cast a 'np:dtype('bool')' to 'np:dtype('float64')'

@matthiasdiener matthiasdiener self-assigned this Jul 18, 2024
@matthiasdiener matthiasdiener requested a review from inducer July 18, 2024 23:06
@inducer
Copy link
Owner

inducer commented Jul 18, 2024

I would much rather allow casting bool to float than revert this change.

@matthiasdiener
Copy link
Collaborator Author

matthiasdiener commented Jul 18, 2024

I would much rather allow casting bool to float than revert this change.

What do you think of 22361dc?

@matthiasdiener matthiasdiener changed the title Revert "Type inference: Use np.bool_ where appropriate, not int32" TypeInferenceMapper: allow np.bool in map_type_case Jul 18, 2024
@inducer
Copy link
Owner

inducer commented Jul 19, 2024

Sure, I could live with that.

@inducer inducer enabled auto-merge (rebase) July 19, 2024 00:49
@inducer inducer merged commit 07cee43 into inducer:main Jul 19, 2024
@matthiasdiener matthiasdiener deleted the revert-bool-logical branch July 19, 2024 14:07
@matthiasdiener matthiasdiener changed the title TypeInferenceMapper: allow np.bool in map_type_case TypeInferenceMapper: allow np.bool in map_type_cast Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@inducer inducer Awaiting requested review from inducer

Assignees

@matthiasdiener matthiasdiener

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@matthiasdiener @inducer