Skip to content

Consistent parameter naming for TransformersReader#510

Merged
tholor merged 2 commits intodeepset-ai:masterfrom
lalitpagaria:reader_parameter_naming_consitancy
Oct 21, 2020
Merged

Consistent parameter naming for TransformersReader#510
tholor merged 2 commits intodeepset-ai:masterfrom
lalitpagaria:reader_parameter_naming_consitancy

Conversation

@lalitpagaria
Copy link
Copy Markdown
Contributor

@lalitpagaria lalitpagaria commented Oct 20, 2020

  • Changing TransformersReader parameter from model to model_name_or_path to make it consistent with FARMReader
  • Removing unused imports as well.

@review-notebook-app
Copy link
Copy Markdown

review-notebook-app Bot commented Oct 20, 2020

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@lalitpagaria
Copy link
Copy Markdown
Contributor Author

lalitpagaria commented Oct 20, 2020

@tholor can you please add breaking change label

@lalitpagaria
Copy link
Copy Markdown
Contributor Author

lalitpagaria commented Oct 20, 2020

Interesting test job is getting killed with error code 137 means docker container killed because OOM -

2020-10-20T17:08:37.1255514Z test_eval.py ...........                                                 [ 56%]
2020-10-20T17:09:28.3133855Z test_faiss.py .........                                                  [ 63%]
2020-10-20T17:17:18.4233618Z /home/runner/work/_temp/a2641cfd-a0dc-4e88-8daa-f54674f5708c.sh: line 1:  4065 Killed                  pytest
2020-10-20T17:17:18.4253736Z test_finder.py ...................
2020-10-20T17:17:18.6246336Z ##[error]Process completed with exit code 137.

@lalitpagaria
Copy link
Copy Markdown
Contributor Author

lalitpagaria commented Oct 20, 2020

I dont see any option to re-run the build, I think it might be related to some permission.
I don't think this change may cause any memory issue.

tholor
tholor requested changes Oct 21, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@tholor tholor left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @lalitpagaria ! Left minor comments ...

Comment thread docs/v0.4.0/_src/tutorials/tutorials/1.md Outdated
Comment thread tutorials/Tutorial1_Basic_QA_Pipeline.ipynb
@lalitpagaria lalitpagaria requested a review from tholor October 21, 2020 14:28
@lalitpagaria lalitpagaria removed their assignment Oct 21, 2020
tholor
tholor approved these changes Oct 21, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@tholor tholor left a comment

Choose a reason for hiding this comment

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

Looking good! Ready to merge :)

@tholor tholor merged commit 63c1237 into deepset-ai:master Oct 21, 2020
@lalitpagaria lalitpagaria deleted the reader_parameter_naming_consitancy branch October 21, 2020 15:24
@antoniolanza1996 antoniolanza1996 mentioned this pull request Nov 12, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tholor tholor tholor approved these changes

Assignees

No one assigned

Labels

breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lalitpagaria @tholor