Skip to content

Conversation

@golechwierowicz
Copy link
Collaborator

@golechwierowicz golechwierowicz commented Dec 13, 2023

This PR introduces a change fixing a disparity between instructions' precision of XLA, and TorchInductor.

For example:
After calling torch.half() or any other buffer/parameter casting TorchInductor will cast all non-fp16 floating types to fp16, and program input shape will correctly be reflected by this. OTOH XLA needs additional environment variables to perform casting upon lowering.

This PR reads the default precision set by TorchBench models for inference, and training and sets the appropriate env vars.

This PR introduces a change fixing a disparity between
instructions' precision of XLA, and TorchInductor.

For example:
After calling torch.half() or any other buffer/parameter casting
TorchInductor will cast all non-fp16 floating types to fp16, and
program input shape will correctly be reflected by this. OTOH
XLA needs additional environment variables to perform casting
upon lowering.
This PR reads the default precision set by TorchBench models
for inference, and training and sets the appropriate env vars.
@golechwierowicz golechwierowicz force-pushed the olechwierowicz/apply-default-torchbench-precision branch from 36e7cf3 to 42d44eb Compare December 13, 2023 14:02
@cota cota self-requested a review December 13, 2023 16:07
@cota
Copy link
Collaborator

cota commented Dec 13, 2023

Thanks Greg! Please land this and I'll run another nightly with it today.

@golechwierowicz golechwierowicz merged commit a085682 into master Dec 13, 2023
del benchmark
gc.collect()

def apply_default_precision_config(self, test, benchmark):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for finding this and adding it to the benchmarks.
I am wondering if this is the right place to fix this.
Is this something we can add in the dynamo bridge when lowering to XLA?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a workaround. See b/316126405

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we should add these things to benchmarking or if that is just more confusing overall. We should at least add a comment in these cases.

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.

4 participants