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.

Sign up for GitHub

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

Jump to bottom

PT: get_model should be moved into the BaseModel class #3968

Closed
njzjz opened this issue Jul 11, 2024 · 0 comments · Fixed by #4002
Closed

PT: get_model should be moved into the BaseModel class #3968

njzjz opened this issue Jul 11, 2024 · 0 comments · Fixed by #4002
Labels
bug

Comments

@njzjz
Copy link
Member

njzjz commented Jul 11, 2024

Otherwise, after one implements a new model, one has to update this method. This cannot be done with the plugin.
@njzjz njzjz added the bug label Jul 11, 2024
njzjz added a commit to njzjz/deepmd-kit that referenced this issue Jul 22, 2024
@njzjz
feat: add get_model classmethod to BaseModel
0db9747 
Fix deepmodeling#3968. External and new models can implement this method (if different from default) without changing the old `get_model` methods (which cannot be done by a plugin).

Note: I don't modify old `get_model` methods in this PR.
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz njzjz mentioned this issue Jul 22, 2024
Merged
@njzjz njzjz linked a pull request Jul 22, 2024 that will close this issue
feat: add get_model classmethod to BaseModel #4002
Merged
github-merge-queue bot pushed a commit that referenced this issue Jul 24, 2024
@njzjz
feat: add get_model classmethod to BaseModel (#4002)
269ed3e 
Fix #3968. External and new models can implement this method (if
different from default) without changing the old `get_model` methods
(which cannot be done by a plugin).

Note: I don't modify old `get_model` methods in this PR.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Introduced a new method for model instantiation that enhances
flexibility in parameter configuration.
- Improved the model retrieval process to support dynamic model
selection based on specified types.
  
- **Bug Fixes**
- Enhanced control flow to ensure correct model type selection,
addressing potential issues with model retrieval.

- **Refactor**
- Updated existing model retrieval functions to streamline logic and
improve maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz njzjz closed this as completed Jul 24, 2024
mtaillefumier pushed a commit to mtaillefumier/deepmd-kit that referenced this issue Sep 18, 2024
@njzjz
feat: add get_model classmethod to BaseModel (deepmodeling#4002)
0c3ebe3 
Fix deepmodeling#3968. External and new models can implement this method (if
different from default) without changing the old `get_model` methods
(which cannot be done by a plugin).

Note: I don't modify old `get_model` methods in this PR.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Introduced a new method for model instantiation that enhances
flexibility in parameter configuration.
- Improved the model retrieval process to support dynamic model
selection based on specified types.
  
- **Bug Fixes**
- Enhanced control flow to ensure correct model type selection,
addressing potential issues with model retrieval.

- **Refactor**
- Updated existing model retrieval functions to streamline logic and
improve maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add get_model classmethod to BaseModel njzjz/deepmd-kit
1 participant
@njzjz