-
Notifications
You must be signed in to change notification settings - Fork 512
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
Optimize Bert_SQUAD_Interpret #672
Conversation
scripts/install_via_conda.sh
Outdated
@@ -40,7 +40,7 @@ conda install -y -c conda-forge black matplotlib pytest-cov sphinx-autodoc-typeh | |||
# install node/yarn for insights build | |||
conda install -y -c conda-forge yarn | |||
# nodejs should be last, otherwise other conda packages will downgrade node | |||
conda update -y --no-channel-priority -c conda-forge nodejs | |||
conda install -y --no-channel-priority -c conda-forge nodejs=13 |
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.
@vivekmig, @edward-io , this fixes the issue with the nodejs version. There might be better fixes but this fixed it for now.
Issue: #669
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.
Can we peg this to nodejs=14? According to https://nodejs.org/en/about/releases/ NodeJS 14 will be supported until 2022.
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.
Thank you, @edward-io! Created a separate PR (#675) so that we so that we don't have to wait Bert tutorial to be reviewed.
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.
Looks great, thanks for fixing this.
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.
Looks great 👍 !
tutorials/Bert_SQUAD_Interpret.ipynb
Outdated
" input_embeddings = interpretable_embedding.indices_to_embeddings(input_ids, token_type_ids=token_type_ids, position_ids=position_ids)\n", | ||
" ref_input_embeddings = interpretable_embedding.indices_to_embeddings(ref_input_ids, token_type_ids=token_type_ids, position_ids=position_ids)\n", | ||
" input_embeddings = model.bert.embeddings(input_ids, token_type_ids=token_type_ids, position_ids=position_ids)\n", | ||
" #interpretable_embedding.indices_to_embeddings(input_ids, token_type_ids=token_type_ids, position_ids=position_ids)\n", |
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.
nit: Should these commented out lines be removed?
8889939
to
3d767bd
Compare
@NarineK has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Optimizes Bert_SQUAD_Interpret tutorial by:
configure_interpretable_layer
by using forward function with theinputs_embeds
argument.Addresses the improvements discussed in: #624