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

feat: add attentive embeddings #92

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Optimox
Copy link
Collaborator

@Optimox Optimox commented Mar 17, 2020

Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of the pull request.

What kind of change does this PR introduce?

This PR aims at improving attention with embeddings.
As the embedding size grows the attention module becomes less relevant, because attention does not know about what columns are from the same embeddings.

In order to solve this problem, this PR adds a mask post processing that takes (for now) the max of attention given to any embedding (mean could be tried as well).

Does this PR introduce a breaking change?
Everything is internal and invisible to the end users.

What needs to be documented once your changes are merged?

Closing issues

Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if such).

@Optimox Optimox force-pushed the feature/embedding-attention branch 4 times, most recently from 61cbf1f to c8bd369 Compare March 18, 2020 07:43
@Optimox Optimox force-pushed the feature/embedding-attention branch from c8bd369 to 114b383 Compare March 18, 2020 09:55
@athewsey
Copy link
Contributor

Just to point out, the torch-scatter dependency introduced here is making it a bit tricky for me to reproduce the Forest Cover Type tests from #217 because:

  • We seem to need to explicitly select the correct install version for your PyTorch & CUDA combination (so things aren't as simple as pyproject.toml makes it look), and
  • I was testing on a PyTorch v1.4 env with CUDA v10.2, which isn't a supported combination for torch-scatter 😞

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

Successfully merging this pull request may close these issues.

None yet

3 participants