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

placeholder from Speechllm selene to main #13

Closed
wants to merge 51 commits into from
Closed

Conversation

zhehuaichen
Copy link
Owner

What does this PR do ?

Add a one line overview of what this PR aims to accomplish.

Collection: [Note which collection this PR will affect]

Changelog

  • Add specific line by line info of high level changes in this PR.

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this 

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

  • New Feature
  • Bugfix
  • Documentation

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

  • Related to # (issue)

Zhehuai Chen and others added 30 commits August 3, 2023 09:53
approve the nit change
* Merge heh and zhehuai's initial version of frozen am+llm

The previous differences are summarized here:
https://docs.google.com/document/d/1zNI4hC6vJtUfcHbrUSPaMuYWRBQdN_36H0P2NiBiuPY/edit

This PR includes
1. Finish merging the model, dataset, and config code
2. Previous tests are still enabled and passed (prepare_llm_input, training_step,
    validation_step)
3. the example training script with LS960 has been run to make sure the training
pipeline works

The major remaining works are listed here
https://docs.google.com/document/d/1o0AM7v4gcTQkPZjE0Vl9TTX4vYnGTrbXEFGWh0UhGlk/edit#bookmark=id.pzvdadt5oxyw

---------

Co-authored-by: He Huang (Steve) <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: stevehuang52 <[email protected]>
Signed-off-by: stevehuang52 <[email protected]>
Signed-off-by: stevehuang52 <[email protected]>
Signed-off-by: stevehuang52 <[email protected]>
zhehuaichen and others added 18 commits August 23, 2023 11:35
Signed-off-by: stevehuang52 <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
multitask understanding; also fix bleu implementation

Signed-off-by: zhehuaichen <[email protected]>
Signed-off-by: zhehuaichen <[email protected]>
Copy link
Owner Author

@zhehuaichen zhehuaichen left a comment

Choose a reason for hiding this comment

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

left some items to run down

@@ -541,46 +593,88 @@ def inference_epoch_end(self, outputs, mode, data_cfg):
averaged_metric = 0.0 if monitor_mode == 'max' else 1e5

if mode == 'validation':
self.log("validation_loss", averaged_loss)
self.log("validation_loss", averaged_loss, batch_size=1, sync_dist=True)

Choose a reason for hiding this comment

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

may not be necessary, I added batch_size=1, sync_dist=True just because other places have them. it's ok to keep the original code.

def on_test_epoch_end(self):
self.on_inference_epoch_end(self.cfg.data.test_ds)
return super().on_test_epoch_end()
# def on_test_epoch_end(self):

Choose a reason for hiding this comment

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

commented out since they are not actually called in PTL1.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants