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 basic grpc MT server #1807

Merged
merged 44 commits into from
Aug 9, 2021
Merged

Add basic grpc MT server #1807

merged 44 commits into from
Aug 9, 2021

Conversation

ryanleary
Copy link
Contributor

Add readme, server updates

Signed-off-by: Ryan Leary rleary@nvidia.com

Add readme, server updates

Signed-off-by: Ryan Leary <rleary@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Feb 26, 2021

This pull request introduces 5 alerts when merging 619df56 into efaab9e - view on LGTM.com

new alerts:

  • 5 for Unused import

request_strings = [x for x in request.texts]

for batch in batches(request_strings, self._batch_size):
batch_results = self._models[lang_pair].translate(text=batch, source_lang=None, target_lang=None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why source and target lang is None here? This won't use the correct input tokenization and target detokenization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, no good reason. Better question is why doesn't the model already know the source_lang and target_lang if it matters? It's being restored from a nemo model.. that data should already be there.

Copy link
Contributor

Choose a reason for hiding this comment

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

We've added that now and new models won't require this (#1805). But for existing models, and for temporary backward compatibility, the translate method has a source_lang and target_lang that need to be set.

@lgtm-com
Copy link

lgtm-com bot commented Feb 26, 2021

This pull request introduces 5 alerts when merging f044219 into 4d6cad6 - view on LGTM.com

new alerts:

  • 5 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Mar 4, 2021

This pull request introduces 5 alerts when merging 79d9ee7 into 91774a7 - view on LGTM.com

new alerts:

  • 5 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Mar 5, 2021

This pull request introduces 5 alerts when merging 645d84e into 7ecf04f - view on LGTM.com

new alerts:

  • 5 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Mar 8, 2021

This pull request introduces 5 alerts when merging 4064008 into 52c20a1 - view on LGTM.com

new alerts:

  • 5 for Unused import

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
Comment on lines 111 to 113
if not hasattr(model, "src_language") or not hasattr(model, "tgt_language"):
raise ValueError(f"Could not find src_language and tgt_language in model attributes")

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add these to the argparser? Then if a model doesn't have them, it could be updated by the user specified languages?

Copy link
Collaborator

@ericharper ericharper left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

Only some minor comments.

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
@lgtm-com
Copy link

lgtm-com bot commented Aug 9, 2021

This pull request introduces 6 alerts when merging 6b474ed into 241855c - view on LGTM.com

new alerts:

  • 4 for Unused import
  • 2 for Modification of parameter with default

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
@lgtm-com
Copy link

lgtm-com bot commented Aug 9, 2021

This pull request introduces 2 alerts when merging 323d938 into 241855c - view on LGTM.com

new alerts:

  • 2 for Modification of parameter with default

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
@lgtm-com
Copy link

lgtm-com bot commented Aug 9, 2021

This pull request introduces 2 alerts when merging af51a3d into 241855c - view on LGTM.com

new alerts:

  • 2 for Modification of parameter with default

@lgtm-com
Copy link

lgtm-com bot commented Aug 9, 2021

This pull request introduces 2 alerts when merging e3f7898 into 7e6197d - view on LGTM.com

new alerts:

  • 2 for Modification of parameter with default

@ericharper ericharper merged commit 2be5853 into NVIDIA:main Aug 9, 2021
blisc pushed a commit to blisc/NeMo that referenced this pull request Aug 12, 2021
* Add basic grpc MT server

Add readme, server updates

Signed-off-by: Ryan Leary <rleary@nvidia.com>

* style fix

Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>

* fixing license headers

Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>

* Add punctuation model into NMT service

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Fix merge conflicts

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* style fixes to unblock CI

Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>

* Add a Jarvis ASR + NeMo NMT client

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* style fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Refactor gRPC service

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Update license headers

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Update one more license header

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Whitepsace in header

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Style fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Fix grpc requirement

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Update license headers

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Add option to specify src/tgt lang and import fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Style fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Fix unused imports

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Renaming variables

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

Co-authored-by: Ryan Leary <rleary@nvidia.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
Co-authored-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
paarthneekhara pushed a commit to paarthneekhara/NeMo that referenced this pull request Sep 17, 2021
* Add basic grpc MT server

Add readme, server updates

Signed-off-by: Ryan Leary <rleary@nvidia.com>

* style fix

Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>

* fixing license headers

Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>

* Add punctuation model into NMT service

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Fix merge conflicts

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* style fixes to unblock CI

Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>

* Add a Jarvis ASR + NeMo NMT client

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* style fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Refactor gRPC service

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Update license headers

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Update one more license header

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Whitepsace in header

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Style fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Fix grpc requirement

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Update license headers

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Add option to specify src/tgt lang and import fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Style fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Fix unused imports

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Renaming variables

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

Co-authored-by: Ryan Leary <rleary@nvidia.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
Co-authored-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
Signed-off-by: Paarth Neekhara <paarth.n@gmail.com>
jfsantos pushed a commit to jfsantos/NeMo that referenced this pull request Nov 19, 2021
* Add basic grpc MT server

Add readme, server updates

Signed-off-by: Ryan Leary <rleary@nvidia.com>

* style fix

Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>

* fixing license headers

Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>

* Add punctuation model into NMT service

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Fix merge conflicts

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* style fixes to unblock CI

Signed-off-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>

* Add a Jarvis ASR + NeMo NMT client

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* style fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Refactor gRPC service

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Update license headers

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Update one more license header

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Whitepsace in header

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Style fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Fix grpc requirement

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Update license headers

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Add option to specify src/tgt lang and import fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Style fixes

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Fix unused imports

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

* Renaming variables

Signed-off-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>

Co-authored-by: Ryan Leary <rleary@nvidia.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com>
Co-authored-by: Oleksii Kuchaiev <okuchaiev@nvidia.com>
Co-authored-by: MaximumEntropy <sandeep.subramanian.1@umontreal.ca>
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

4 participants