Fix LTC tests on Windows #74960
Fix LTC tests on Windows #74960
Conversation
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit b155c3c (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
test/lazy/test_ts_opinfo.py
Outdated
There was a problem hiding this comment.
aha. so basically the code we use to generate the counter names is behaving differently on windows?
Any idea if we could fix that instead? (to make it behave the same as linux) rather than patching the test to expect different behavior on windows?
https://github.com/pytorch/pytorch/blob/master/torch/csrc/lazy/ts_backend/ts_eager_fallback.cpp#L109
There was a problem hiding this comment.
It's probably related to the name of struct torch::lazy::LazyTensorNativeFunctions. For example, this
at::Tensor LazyNativeFunctions::alias(const at::Tensor& self) {
TORCH_LAZY_FN_COUNTER("lazy::");
return self;
}
is expanded to
#define TORCH_LAZY_FN_COUNTER(ns) \
TORCH_LAZY_COUNTER(c10::str(ns, __FUNCTION__), 1)
I assume __FUNCTION__ is mapped to fully-qualified name.
There was a problem hiding this comment.
Yea, it seems like FUNCTION expands to alias on linux but torch::lazy::LazyTensorNativeFunctions:alias on windows. We actually don't want the full name for our purposes (having the counter named torch::lazy::LazyTensorNativeFunctions:alias is not ideal).
I wonder if there is some msvc trick or another preprocessor directive we could use so that we can get just the local function name without namespace on both linux/windows?
There was a problem hiding this comment.
Maybe this can help.
#define TORCH_LAZY_FN_COUNTER(ns) \
TORCH_LAZY_COUNTER(c10::str(ns, __func__), 1)
Basically, __func__ is defined in C/C++ standard so we should use __func__ instead of non-standardized __FUNCTION__ (which has different implementations in different compilers).
|
Cool, the |
|
@wconstab has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
wschin
force-pushed
the
fix-ltc-win-ltc-test
branch
2 times, most recently
from
60104f0 to
93011f6
Compare
wschin
force-pushed
the
fix-ltc-win-ltc-test
branch
from
93011f6 to
b155c3c
Compare
|
@wconstab has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Hey @wschin. |
4 participants
Fixes #74519