Skip to content

[chat] remove naive strategy and split colossalai strategy#4094

Merged
ver217 merged 10 commits intohpcaitech:mainfrom
cwher:a-tale-of-two-strategies
Jun 29, 2023
Merged

[chat] remove naive strategy and split colossalai strategy#4094
ver217 merged 10 commits intohpcaitech:mainfrom
cwher:a-tale-of-two-strategies

Conversation

@cwher
Copy link
Copy Markdown
Contributor

@cwher cwher commented Jun 27, 2023

📌 Checklist before creating the PR

  • I have created an issue for this PR for traceability
  • The title follows the standard format: [doc/gemini/tensor/...]: A concise description
  • I have added relevant tags if possible for us to better distinguish different PRs

🚨 Issue number

Link this PR to your issue with words like fixed to automatically close the linked issue upon merge

e.g. fixed #1234, closed #1234, resolved #1234

Close #4084 and #4085.

📝 What does this PR do?

Summarize your work here.
if you have any plots/diagrams/screenshots/tables, please attach them here.

💥 Checklist before requesting a review

  • I have linked my PR to an issue (instruction)
  • My issue clearly describes the problem/feature/proposal, with diagrams/charts/table/code if possible
  • I have performed a self-review of my code
  • I have added thorough tests.
  • I have added docstrings for all the functions/methods I implemented

⭐️ Do you enjoy contributing to Colossal-AI?

  • 🌝 Yes, I do.
  • 🌚 No, I don't.

Tell us more if you don't enjoy contributing to Colossal-AI.

@cwher cwher added the chatgpt ChatGPT Application label Jun 27, 2023
@cwher cwher marked this pull request as ready for review June 28, 2023 03:59
@cwher cwher force-pushed the a-tale-of-two-strategies branch from 8e459b9 to 9668432 Compare June 29, 2023 02:53
@cwher cwher force-pushed the a-tale-of-two-strategies branch from 9668432 to 2e259c3 Compare June 29, 2023 03:05
@cwher cwher requested a review from ver217 June 29, 2023 04:29
ver217
ver217 reviewed Jun 29, 2023
View reviewed changes
Comment thread applications/Chat/coati/trainer/strategies/ddp.py Outdated
@cwher cwher linked an issue Jun 29, 2023 that may be closed by this pull request
[chat] split colossalai strategy into gemini and low level zero strategy #4085
Closed
1 task
@cwher cwher requested a review from ver217 June 29, 2023 08:43
@ver217 ver217 merged commit edd75a5 into hpcaitech:main Jun 29, 2023
ver217 pushed a commit to ver217/ColossalAI that referenced this pull request Jul 13, 2023
@cwher @ver217
[chat] remove naive strategy and split colossalai strategy (hpcaitech…
d7a15c9 
…#4094)

* feat: remove on_learn_epoch fn as not used

* revert: add _on_learn_epoch fn

* to: remove the use of NaiveStrategy

* test: remove NaiveStrategy tests

* feat: remove NaiveStrategy

* style: modify comments and params

* feat: split ColossalAIStrategy into LowLevelZeroStrategy and GeminiStrategy

* fix: remove naive

* fix: align with modified colossal strategy

* fix: fix ddp _try_init_dist arg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ver217 ver217 ver217 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

chatgpt ChatGPT Application

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[chat] split colossalai strategy into gemini and low level zero strategy [chat] remove naive strategy

2 participants

@cwher @ver217