-
Notifications
You must be signed in to change notification settings - Fork 36
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 ability to switch output languages for multilingual models #69 #72
Closed
Closed
Changes from 23 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
89fbc24
Testing Circleci on main
93f9643
Merge branch 'main' of https://github.com/SamDewriter/SimulEval
fd41dc2
Testing Circleci on main
4becf02
Testing Circleci on main
cc893ef
Testing Circleci on main
a6f00f8
Testing Circleci on main
0f41351
correct Circle config
78d13d0
correct Circle config
98ae6ec
correct Circle config
cdffc8d
correct Circle config
cacfbc9
Revert "[demo] s2t + s2s agent pipelines (#58)"
f97cdfa
resolve branch changes
4ddf84a
add target language
69e5816
add target language as a parameter
812bcbc
Test dynamic language
233aa35
Switch language dynamically
f15634c
Add ability to switch output language (#69)
c06fb0c
Add tgt language argument
5e0165a
Add Namespace to args argument (#69)
c43e9da
Modify code to read target language from a file
bbe6a88
Add ability to switch input language
03c06b1
Add a tgt-lang file to test
9d97d18
Add tgt_lang to instance
3e5fbe6
Add tgt_lang to AgentStates
6d06a8e
States
50efb42
Add tgt_lang from state to test (#69)
1c07b35
Target language to test
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
name: TestPyPI Publish | ||
|
||
on: | ||
push: | ||
branches: | ||
- main | ||
|
||
jobs: | ||
test_pypi_publish: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v2 | ||
|
||
- name: Set up Python | ||
uses: actions/setup-python@v2 | ||
with: | ||
python-version: 3.x | ||
|
||
- name: Install dependencies | ||
run: | | ||
sudo apt-get update | ||
sudo apt-get install -y python3-pip | ||
pip install pipenv | ||
pipenv install twine | ||
|
||
- name: Build and Publish | ||
env: | ||
TWINE_USERNAME: __token__ | ||
TWINE_PASSWORD: ${{ secrets.TEST_PYPI_TOKEN }} | ||
REPOSITORY_URL: https://test.pypi.org/legacy/ | ||
run: | | ||
python setup.py sdist bdist_wheel | ||
pipenv run twine upload --repository-url $REPOSITORY_URL dist/* |
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
10 changes: 0 additions & 10 deletions
10
examples/speech_to_speech_demo/english_counter_pipeline.py
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is this part here as a fallback? It is fine to leave it in as a fallback, but if this is the sole logic for getting tgt_lang in this agent then it won't meet our purpose.
We primarily want to get the tgt lang param dynamically passed to the policy method instead of solely getting set at initialization. You understand that solely getting set at initialization means it will remain static, right?
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.
The reason I added this part is just to preprocess the tgt_lang because we are reading it from a file. I came to this conclusion after several trials and errors. I noticed that before adding the logic, the evaluation works well but the tgt_lang is not being passed.
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.
One thing I'd also need clarification on is that in the counter_in_tgt_lang_agent, the tgt_lang is being passed to the class after inheriting from the parent class Agent, which means that the tgt_lang is not implemented in the parent class. I suppose the right thing will be to implement it in the parent class so that all the children classes will automatically have it
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.
We don't want to set tgt_lang in the init method by the end of this effort, so don't worry about moving this attribute to the parent class for now.
If the objective is still not clear to you, imagine that
SimulEval/examples/speech_to_text/counter_in_tgt_lang_agent.py
Lines 22 to 24 in c7a1749
SimulEval/examples/speech_to_text/counter_in_tgt_lang_agent.py
Line 17 in c7a1749
policy
method as part of thestates
, something like: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.
Oh! Now I understand the goal. Thanks for the clarification!