Skip to content

Dtype for reduction#361

Merged
jjsjann123 merged 4 commits into20_8_18_develfrom
dtype_for_reduction
Sep 14, 2020
Merged

Dtype for reduction#361
jjsjann123 merged 4 commits into20_8_18_develfrom
dtype_for_reduction

Conversation

@jjsjann123
Copy link
Copy Markdown
Collaborator

Fixes #357

Two things in this PR:

  1. Do type propagation even for profiling executor -> this is the root cause for bug reported in FP16 reductions are returning Float instead of FP16 #357
  2. Allow dtype argument in sum, which is simply handled in our type propagation.

@jjsjann123
Copy link
Copy Markdown
Collaborator Author

I'm hitting a kernel issue with the support. fp16 kernel is giving me wrong output, looks like predicate is wrong. I'll open another issue to track it.

const auto out_type = node->input(0)->type()->cast<TensorType>();
auto out_type = node->input(0)->type()->cast<TensorType>();

if (!node->input(3)->type()->isSubtypeOf(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add a comment explaining what's going on?

Comment thread torch/csrc/jit/codegen/cuda/parser.cpp Outdated
static_cast<c10::TypePtr>(NoneType::get()))) {
// We can only handle output as half and float;
if (auto opt_ivalue = toIValue(node->input(3))) {
auto scalar_type = opt_ivalue->toScalarType();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

const?

// back to fp16.
TypePropagate(graph);
}
TypePropagate(graph);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is this still related to the corner case described in the original comment? if that's the case maybe we should preserve the comment?

@jjsjann123 jjsjann123 merged commit 0f330c3 into 20_8_18_devel Sep 14, 2020
@jjsjann123 jjsjann123 deleted the dtype_for_reduction branch September 14, 2020 19:22
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.

FP16 reductions are returning Float instead of FP16

2 participants