Add singularity function for maximum and minimum #1642
Conversation
|
|
||
| def min_max_singularity_fn_producer(sample): | ||
| a, b = sample.args | ||
| if a.shape == b.shape or b.shape == (): |
There was a problem hiding this comment.
Would you add a comment explaining what's happening with minimum and maximum and singularities?
| @@ -208,10 +198,24 @@ def _to(x): | |||
| args, kwargs = tree_map(_to, self.args), tree_map(_to, self.kwargs) | |||
| return SampleInput(*args, **kwargs) | |||
|
|
|||
| def remove_singularities(self, singularity_fn, eps): | |||
| def remove_singularities(self, op, eps): | |||
There was a problem hiding this comment.
Would you add a comment about the current singularity specifying patterns and how they work? There have been a few changes.
One thing that'd be neat in the future is if a SampleInput could be bound to a signature, like
ba = si.bind_for(ltorch.maximum)
Where ba could be a BoundArguments object (https://docs.python.org/3/library/inspect.html#inspect.BoundArguments), so that the args and kwargs in the sample input could be accessed in some canonical fashion. That would make it easier to inspect and manipulate the sample inputs.
We might also want to replace singularity_fn_producer with something like singularity_handler that just takes a SampleInput and maybe returns a different or modified SampleInput, with complete control over its analysis. That might make the maximum and minimum stuff more readable.
What are your thoughts, @beverlylytle?
Flakiness has recently been observed in maximum. The provided logs do not give a clear indication of the cause of the failure. However, the test for
maximumcan be induced to fail when one entry of one tensor is very near the corresponding entry of the other tensor. The rarity of such an event for these randomly generated tensors is consistent with the rarity of failure for these tests. This PR introduces a singularity function for these operators to avoid such a situation.