-
Notifications
You must be signed in to change notification settings - Fork 512
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
validate forward_fun output shape in FeatureAblation #1091
Conversation
), ( | ||
"When perturbations_per_eval > 1, forward_func's output " | ||
"should be a tensor whose 1st dim grow with the input " | ||
f"batch size: when input batch size is {num_examples}, " |
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 think in future we can be more strict here by requiring the 1st dim of output to be the same as input batch size, instead of just require it "grow with the input batch size".
For example, if the input batch is 2
, the output's 1st dim can be 2
, 4
, or 7
; currently it will be OK
when we expand the input batch size to 4
, the output's 1st dim becomes 4
, 8
, or 14
respectively.
But the last 2 cases are pretty weird and unlikely to happen. It may be easier to always require output's 1st dim to be batch size and it must be the same as input batch size in this "non-aggregation mode". So if the input batch is 2
, the output's 1 dim must be 2
if perturbations_per_eval > 1
@aobo-y has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
return inp[0].unsqueeze(0) | ||
|
||
inp = torch.tensor([[1, 2, 3], [4, 5, 6]]) | ||
mask = torch.tensor([[0, 1, 2], [0, 0, 1]]) |
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.
This test checks when rowwise mask
is given, the model must not be in "aggregated mode". But I don't think mask
has anything to do with it. Models of both batch-mode and aggregated-mode may need specify mask
. So I simply deleted this test.
@aobo-y has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@aobo-y has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Correctly validate the
forward_fun
's output shape inFeatureAblation
(andFeaturePermutation
)Abandoned previous flawed assumption of "aggregation mode", which forbid the support for multi-outputs models (ref #1047)
New logic does not care output shape when
perturbations_per_eval == 1
. Only whenperturbations_per_eval > 1
, it require "Non-Aggregation mode", which is defined as the 1st dim of the model's output should grow with the input's batch size in the same ratio. This is achieved by actually comparing the output shape of 2 different inputs instead of making any assumption based on other user config:This way does not even introduce any extra forward calls.