Skip to content
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

Array indexing with list of strings. #35

Closed
franciscomalveiro opened this issue Apr 14, 2023 · 1 comment
Closed

Array indexing with list of strings. #35

franciscomalveiro opened this issue Apr 14, 2023 · 1 comment
Assignees

Comments

@franciscomalveiro
Copy link

Hello! First of all thanks for your work on the development of this project. I have been trying to use your code to generate explanations for an LSTM model.

I have been following the notebook you provide and adapting it to my use case.

To generate global feature explanations, you make use of the function

def feat_explain_all(f: Callable,
data: Union[List[np.ndarray], pd.DataFrame, np.array],
feat_dict: dict,
pruning_data: pd.DataFrame,
baseline: Union[pd.DataFrame, np.array] = None,
model_features: List[Union[int, str]] = None,
schema: List[str] = None,
entity_col: Union[int, str] = None,
time_col: Union[int, str] = None,
append_to_files: bool = False,
verbose: bool = False,
) -> pd.DataFrame:

I have been calling with with the arguments suited to my use case, specifically providing it the model_features
argument (a list of strings)

model_features: List[str]
Features to be used by the model. Requires same order as training dataset

and it generates the following error:

line 327, in feat_explain_all
    sequence = sequence[:, :, model_features]
IndexError: only integers, slices (`:`), ellipsis (`...`), numpy.newaxis (`None`) and integer or boolean arrays are valid indices

This is the line of code:

if model_features:
sequence = sequence[:, :, model_features]

I have noticed that you create indices above and never use the variable model_features_index

model_features_index, entity_col_index, time_col_index = convert_to_indexes(model_features, schema, entity_col, time_col)

It makes sense that the array should be accessed with these indexes, rather than with a list of strings, is that it?

@JoaoPBSousa
Copy link
Collaborator

Hello @franciscomalveiro,

Thank you for noting this issue. You are indeed correct. We should use the model_features_index variable instead of model_features.

We already pushed a new version of TimeSHAP with this fixed.

@JoaoPBSousa JoaoPBSousa self-assigned this May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants