Skip to content

Conversation

@joshua-xia
Copy link

fix this issue if the inputs is list, and convert the list to tuple, the fix has been verified on my testcase

…type torch.Tensor but <class 'list'> found: error happened
@aobo-y
Copy link
Contributor

aobo-y commented Nov 28, 2022

Hi @joshua-xia , why do you need this change?

We do not plan to expand the support of input types for now. The inputs can be a single tensor or a series of tensor. If the inputs is a series of tensors, Captum expects it to be passed in as Tuple. list conversion does not bring much of convenience. It's easier for us to have strict typing within Captum.

But the error message can be improved to explicitly mention Tuple. We will fix it.

@joshua-xia
Copy link
Author

Hi @joshua-xia , why do you need this change?

We do not plan to expand the support of input types for now. The inputs can be a single tensor or a series of tensor. If the inputs is a series of tensors, Captum expects it to be passed in as Tuple. list conversion does not bring much of convenience. It's easier for us to have strict typing within Captum.

But the error message can be improved to explicitly mention Tuple. We will fix it.

we have the multi-parameters forward function as:

def forward(self, input_ids, token_type_ids=None, attention_mask=None, labels=None):

The TCAV of captum must be comptiable to this kind of forward function.

Thanks!

@NarineK
Copy link
Contributor

NarineK commented Nov 30, 2022

@joshua-xia, is input_ids a list for you ? We expect forward pass to take a tensor or a tuple or tensors as first argument, if some of the inputs following input_ids aren't tensors then you could try passing them as additional_forward_args:
https://captum.ai/api/concept.html
You can also have a wrapper model that is taking tensor or a tuple of tensors and converting it back to a list in the forward method.
If you share your notebook with us we can try to get it working for you.

@joshua-xia
Copy link
Author

@joshua-xia, is input_ids a list for you ? We expect forward pass to take a tensor or a tuple or tensors as first argument, if some of the inputs following input_ids aren't tensors then you could try passing them as additional_forward_args: https://captum.ai/api/concept.html You can also have a wrapper model that is taking tensor or a tuple of tensors and converting it back to a list in the forward method. If you share your notebook with us we can try to get it working for you.

input_ids, token_type_ids, attention_mask, labels can all be list or at least one is list.
sorry for the code from company is NOT allowed to upload to public github.

facebook-github-bot pushed a commit that referenced this pull request Dec 8, 2022
Summary:
Discussed in #1077 (comment) , improving the error message of invalid `inputs` to tell the two types we allow: `Tensor` and `tuple[Tensor]`

Pull Request resolved: #1083

Reviewed By: vivekmig

Differential Revision: D41795064

Pulled By: aobo-y

fbshipit-source-id: 16370e747d08ec33070fc039d9ee2b6dde033cc8
@aobo-y
Copy link
Contributor

aobo-y commented Dec 22, 2022

Captum has no issue with multi-parameter inputs. We just require they are formatted as tuple instead of list. We have corrected the error message accordingly. For list inputs, a simple wrapper can solve the issue

inputs = (input_1, input_2)

def wrapper(*args):
  return your_model(list(args))
  # your model will receive [input_1, input_2]

Let us know if this does not solve your problem. We can reopen this. It is better if you can provide us some code showing how you are using it. We don't need your company code, just pseudo code showing us the interface and data type.

@BinchaoPeng
Copy link

Captum has no issue with multi-parameter inputs. We just require they are formatted as tuple instead of list. We have corrected the error message accordingly. For list inputs, a simple wrapper can solve the issue

inputs = (input_1, input_2)

def wrapper(*args):
  return your_model(list(args))
  # your model will receive [input_1, input_2]

Let us know if this does not solve your problem. We can reopen this. It is better if you can provide us some code showing how you are using it. We don't need your company code, just pseudo code showing us the interface and data type.

Hi, if the input of my model requires seqs such as "hello, bro!", can I use a wrapper or other ways to solve the issue that the first param of ig.att must be a tensor? @NarineK @aobo-y

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants