Unify the logics to check eager mode #7709
Conversation
|
Do you think its valuable to add a message letting the users know that eager mode run without torch.compile is going to be slow, can provide some basic reasoning as to why that would be the case. |
|
@aws-rhsoln check my doc at #7700, I am still thinking about how to message users better to let them know eager mode exists. but yea I agree that we have to make it clear that compile is the way to go for perfomrance. |
|
@aws-rhsoln for some reason I can't make you a reviewer, do you mind also taking a look at this pr? |
| @@ -243,6 +243,10 @@ def _init_xla_lazy_backend(): | |||
| plugins.use_dynamic_plugins() | |||
| plugins.register_installed_plugins() | |||
|
|
|||
| if os.getenv('XLA_USE_EAGER_DEBUG_MODE', '0') == '1': | |||
There was a problem hiding this comment.
Is this just for backward compatibility, since we already have the api now?
There was a problem hiding this comment.
yea, it is mostly for backward compatibility. I think I can also tweak it and use it in test to test both eager and non-eager.
torch_xla/csrc/tensor.cpp
Outdated
| @@ -86,8 +86,7 @@ XLATensorPtr XLATensor::Create( | |||
| XLATensor(std::move(ir_value), device, logical_element_type)); | |||
| XLAGraphExecutor* graph_executor = XLAGraphExecutor::Get(); | |||
| graph_executor->RegisterTensor(xtensor->data()); | |||
| if ((UseEagerDebugMode() || graph_executor->UseEagerMode()) && | |||
| !delay_eager_executation) { | |||
| if ((graph_executor->UseEagerMode()) && !delay_eager_executation) { | |||
There was a problem hiding this comment.
Nit: Extra brackets around graph_executor->UseEagerMode()
Prior to my eager mode changes, eager mode is a debug only mode that's controlled by the
XLA_USE_EAGER_DEBUG_MODEenv var, now since we want to make it more offical I want to control it explictly with the API instead of the env var. @aws-rhsoln let me know what you think. Eventually I want to deprecate the env var(at least not call it debug mode).