-
Notifications
You must be signed in to change notification settings - Fork 48
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
Proposal for Multimodal Large Model Joint Learning Algorithm: Reproduction Based on KubeEdge-Ianvs #167
Conversation
Signed-off-by: Aryan Yadav <[email protected]>
Signed-off-by: Aryan Yadav <[email protected]>
Signed-off-by: Aryan Yadav <[email protected]>
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.
This proposal is developed after the discussion on #163. Considering the limited time of LFX, it is suggested to build based on the existing dataset and datatype.
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 proposal overall is fine. Given that the dataset is an existing one in KubeEdge Community (could be provided from @hsj576 ), besides introducing the dataset, the proposal could be further improved by adding the specific modification on ianvs.
That is, what would this proposal aim to add in KubeEdge Ianvs? For example, there will be changes on example/xx/testenv/xxmetric.py, or example/aaa/bb.yaml. That would help the community to understand what would this proposal bring to ianvs.
Signed-off-by: Aryan Yadav <[email protected]>
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.
-
There are a few comments to discuss what would be expected in this proposal.
Ref: Proposal for Multimodal Large Model Joint Learning Algorithm: Reproduction Based on KubeEdge-Ianvs #167 (review) -
This pull request contains 4 commits with vague commit messages (like "updated"), which might make maintenance difficult, especially considering the number of contributors, pull requests, and their commits in KubeEdge Ianvs recently. After all the comments are tackled, in the final stage, @aryan0931 can squash the commits into one using rebase techniques. Besides, a suggested commit message would be "add proposal of xxx"
docs/proposals/scenarios/Cloud_robotics/single_task_learning.md
Outdated
Show resolved
Hide resolved
docs/proposals/scenarios/Cloud_robotics/single_task_learning.md
Outdated
Show resolved
Hide resolved
docs/proposals/scenarios/Cloud_robotics/single_task_learning.md
Outdated
Show resolved
Hide resolved
Signed-off-by: Aryan Yadav <[email protected]>
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.
This version looks good to me. It proposes to add MLM in the ianvs/example/Cloud_Robotics. The timely works and revisions from @aryan0931 is appreciated. Looking forward to see future implementations~
/lgtm |
We need another lgtm to go. @hsj576 might want to take a look at the proposal. |
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.
/lgtm
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.
All reviewer comments are tackled. Good job!
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hsj576, MooreZheng The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Proposal for Multimodal Large Model Joint Learning Algorithm: Reproduction Based on KubeEdge-Ianvs
Which issue(s) this PR fixes:
Fixes #123