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

Support intern lanuage model #51

Merged
merged 11 commits into from
Jul 27, 2023

Conversation

go-with-me000
Copy link
Collaborator

To support the evaluation and loading of internal models.

Before PR:

[y] Pre-commit or other linting tools are used to fix the potential lint issues.
[y] Bug fixes are fully covered by unit tests, the case that causes the bug should be added in the unit tests.
[y] The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
[y] The documentation has been modified accordingly, like docstring or example tutorials.
After PR:

[y] If the modification has potential influence on downstream or other related projects, this PR should be tested with those projects.
[y] CLA has been signed and all committers have signed the CLA in this PR.

from opencompass.models.base import BaseModel, LMTemplateParser
from opencompass.registry import MODELS

sys.path.append(os.getcwd() + '/InternLM/')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd suggest moving this line into the InternLM.init function, so that sys.path would be modified only when necessary. Otherwise, it may result in some unexpected behavior for general users who don't need this repo at all.



@MODELS.register_module(name=['intern_model'])
class intern_model(BaseModel):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The name of the class should be in the camel case.

e.g. InternLM

sys.path.append(os.getcwd() + '/InternLM/')


@MODELS.register_module(name=['intern_model'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Delete this line, we are using new config system and do not need Registry any more

model_config: Optional[str] = None,
tokenizer_type: Optional[str] = 'v7',
meta_template: Optional[Dict] = None):
root = os.getcwd() + '/opencompass/models/intern/model/'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Never hardcode anything. Just let users specify it in the parameter.

from opencompass.models.base import BaseModel, LMTemplateParser


class internLM(BaseModel):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
class internLM(BaseModel):
class InternLM(BaseModel):

Comment on lines 6 to 8
from .datasets.nq.nq_gen import nq_datasets

datasets = [*nq_datasets]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
from .datasets.nq.nq_gen import nq_datasets
datasets = [*nq_datasets]
from .datasets.collections.base_medium import datasets

Like how it was done in
https://github.com/InternLM/opencompass/blob/main/configs/eval_internlm_7b.py

datasets = [*nq_datasets]

@gaotongxiao gaotongxiao merged commit 57fcfc9 into open-compass:main Jul 27, 2023
1 check passed
go-with-me000 added a commit to go-with-me000/opencompass that referenced this pull request Oct 9, 2023
* support internLM

* support internLM

* simplify intern model files

* update storage_manager

* support internLM

* Modify the file organization structure

* support internLM

* support internLM

* support internLM

* support internLM

* change some details
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

2 participants