-
Notifications
You must be signed in to change notification settings - Fork 40
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.
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
Hide PyTorch trace compilation warnings #185
Conversation
c45fcc4
to
3b5f6c8
Compare
3b5f6c8
to
fb32846
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor comment since ideally I would avoid having the same function defined in two places but probably unavoidable here due to imports?
torchquad/integration/monte_carlo.py
Outdated
# The PyTorch trace compilation warnings contain many false | ||
# positives, so we hide all trace compiler warnings | ||
def trace_without_warnings(*args, **kwargs): | ||
with warnings.catch_warnings(): | ||
warnings.filterwarnings( | ||
"ignore", category=torch.jit.TracerWarning | ||
) | ||
return torch.jit.trace(*args, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you are not making this function a dedicated file in the utils
folder to avoid the torch import if possible, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved it to the utils.py
file now, which also contains helper functions.
fb32846
to
f561a61
Compare
I think the function can be moved into the |
The test execution shows warnings about traces being potentially incorrect because the Python3 control flow is not completely recorded. This includes conditions on the shape of the integration domain tensor. Since the only arguments of the compiled integration function are the integrand and integration domain, and the dimensionality of this integration domain is constant, we can ignore the warnings. After this change, the two `get_jit_compiled_integrate` functions hide PyTorch trace compilation warnings with `warnings.filterwarnings`.
f561a61
to
b088aec
Compare
Description
The test execution shows warnings about traces being potentially incorrect because the Python3 control flow is not completely recorded.
This includes conditions on the shape of the integration domain tensor.
Since the only arguments of the compiled integration function are the integrand and integration domain,
and the dimensionality of this integration domain is constant,
we can ignore the warnings.
After this change, the two
get_jit_compiled_integrate
functions hide PyTorch trace compilation warnings withwarnings.filterwarnings
.Alternatively, it is possible to hide the warnings only at the code locations where they appear, but this would significantly increase the code complexity I think, see for example this diff to hide a subset of the warnings.
Resolved Issues
How Has This Been Tested?
Execution of pytest before and after the changes