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

Add legend for visualize_text #403

Closed
wants to merge 9 commits into from

Conversation

p16i
Copy link
Contributor

@p16i p16i commented Jun 11, 2020

Related to #401.

The PR adds an option for the user to show the legend for visualize_text.

image

@p16i p16i marked this pull request as draft June 11, 2020 12:46
Copy link
Contributor

@NarineK NarineK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this PR, @heytitle ! I left one small comment!

dom.append('<b>Legend: </b>')

for value, label in zip(
[-1, 0, 1], ["Negative", "Neutral", "Positive Attribution"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this PR @heytitle ! It feels a little bit that Attribution is related to positive only. Perhaps leaving it just ["Negative", "Neutral", "Positive"] would be better and the fact that is attribution is intuitive ?

@p16i
Copy link
Contributor Author

p16i commented Jun 12, 2020

@NarineK thanks for the comment! I haven't thought about that. I thought attribution can be associated to negative and positive.

Anyway, I've updated the PR.

@p16i
Copy link
Contributor Author

p16i commented Jun 16, 2020

@NarineK anything else we should do here apart from fixing the lint issues?

@NarineK
Copy link
Contributor

NarineK commented Jun 19, 2020

Looks great! I'd update the branch. It looks like the branch is out of date :) Thank you :)

vivekmig and others added 5 commits June 20, 2020 09:59
Summary:
Disable pytext install in pip builds to avoid segfault
Pull Request resolved: pytorch#404

Reviewed By: bilalsal

Differential Revision: D21998560

Pulled By: vivekmig

fbshipit-source-id: 16664da0541a998f7d9e54cd6edc0ac7705f0619
…available() up to module level (pytorch#402)

Summary:
Pull Request resolved: pytorch#402

In this diff we attempt to prevent listing of the tests in `test_jit.py` and `test_data_parallel.py` if CUDA is not available by moving the CUDA check up to the module level. This way we don't have to discover these tests and then attempt to run them only for it to trigger this exact code path somewhere down the setup stage for each test case.

The main reason why this takes so long is that the computation for `torch.cuda.is_available()` requires the import of the module `torch._C` which we've profiled in the past to take anywhere between 15-20 seconds. To do this for over 1000 tests every single time we test `//pytorch/captum:attributions` is expensive (both timewise and resource wise).

 ---

Reviewed By: vivekmig

Differential Revision: D21966198

fbshipit-source-id: 913173879582df562a188be16560ee60e183b084
Summary:
Some housekeeping to prepare for future changes.

- Separate out React components into individual files
- Switch from class based to function based components
- Fix accessibility lint

To test:

Build the frontend code and use the Insights example. Additionally test using notebook.
Pull Request resolved: pytorch#400

Differential Revision: D21969379

Pulled By: edward-io

fbshipit-source-id: c8632359877df39a373b20bda2a03a3241163275
…ytorch#406)

Summary:
Pull Request resolved: pytorch#406

Hide features that don't generate meaningful interpretable features, e.g. embeddings from other models.

Reviewed By: vivekmig

Differential Revision: D22024855

fbshipit-source-id: f45c8fdbcad7161155545c927777d6c16c5b7349
@p16i p16i force-pushed the legend-for-visualize-text branch from e48226b to 7a28dea Compare June 20, 2020 07:59
@p16i p16i marked this pull request as ready for review June 20, 2020 08:50
@p16i
Copy link
Contributor Author

p16i commented Jun 20, 2020

There is one failed test due to time exceeded. I'm not sure how I should fix it. Any suggestion?

@NarineK Any further comment on this?

@NarineK
Copy link
Contributor

NarineK commented Jul 21, 2020

@heytitle , thank you for the PR! Do you mind updating the branch to fix failing CircleCI!

@p16i
Copy link
Contributor Author

p16i commented Jul 22, 2020

@NarineK the test didn't succeed because it was timeout. Any suggestion on fixing the issue?

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NarineK has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@NarineK
Copy link
Contributor

NarineK commented Jul 27, 2020

@heytitle , I rerun the tests and it looks like there are some unrelated test errors. We actually fixed them and if you update your branch they should be gone. Do you mind updating the branch with the latest changes ?

@p16i
Copy link
Contributor Author

p16i commented Jul 27, 2020

@NarineK I guess it's good to be merged now :)

@facebook-github-bot
Copy link
Contributor

@NarineK merged this pull request in e215a5a.

NarineK pushed a commit to NarineK/captum-1 that referenced this pull request Nov 19, 2020
Summary:
Related to pytorch#401.

The PR adds an option for the user to show the legend for `visualize_text`.

<img width="643" alt="image" src="https://user-images.githubusercontent.com/1214890/84386814-45d14780-abf2-11ea-8a5d-6445186597b8.png">

Pull Request resolved: pytorch#403

Reviewed By: bilalsal

Differential Revision: D22758674

Pulled By: NarineK

fbshipit-source-id: e8d3c7b222098a59f249e44b0f503230d1ca177e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants