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

[BUG] Finetuned model has wrong type_map #3455

Closed
iProzd opened this issue Mar 13, 2024 · 8 comments · Fixed by #3803
Closed

[BUG] Finetuned model has wrong type_map #3455

iProzd opened this issue Mar 13, 2024 · 8 comments · Fixed by #3803
Assignees
Labels
Milestone

Comments

@iProzd
Copy link
Collaborator

iProzd commented Mar 13, 2024

Bug summary

When doing finetuing, the user-defined type_map (e.g. ['H', 'O']) will be covered by the type_map in the pretrained model (e.g. the whole periodic table) , which is confusing for users.

DeePMD-kit Version

3.0.0a

TensorFlow Version

2.6.0

How did you download the software?

Built from source

Input Files, Running Commands, Error Log, etc.

See above.

Steps to Reproduce

See above.

Further Information, Files, and Links

No response

@iProzd iProzd added the bug label Mar 13, 2024
@iProzd iProzd self-assigned this Mar 13, 2024
@njzjz
Copy link
Member

njzjz commented Mar 13, 2024

Idea: the easiest way is to add a virtual Model, "adapt type map model", which just adapts the input atom_type from the outer model type_map to the inner model type_map and forwards everything else like #3450.

@iProzd
Copy link
Collaborator Author

iProzd commented Mar 13, 2024

Idea: the easiest way is to add a virtual Model, "adapt type map model", which just adapts the input atom_type from the outer model type_map to the inner model type_map and forwards everything else like #3450.

I see, but this will cause the model to be wrapped repeatedly each time it is fine-tuned, as discussed with @wanghan-iapcm and @anyangml.

@njzjz
Copy link
Member

njzjz commented Mar 13, 2024

repeatedly each time it is fine-tuned

Indeed, I don't understand why it needs to change the type map each time it is fine-tuned...

@anyangml
Copy link
Collaborator

repeatedly each time it is fine-tuned

Indeed, I don't understand why it needs to change the type map each time it is fine-tuned...

For the LinearModel, let's say we have two pre-trained models: model A with ["H", "O", "Na"], model B with ["H", "O", "K"]. Now if we want to finetune a LinearModel with this, the new type map becomes ["H", "O"].

@njzjz
Copy link
Member

njzjz commented Mar 13, 2024

repeatedly each time it is fine-tuned

Indeed, I don't understand why it needs to change the type map each time it is fine-tuned...

For the LinearModel, let's say we have two pre-trained models: model A with ["H", "O", "Na"], model B with ["H", "O", "K"]. Now if we want to finetune a LinearModel with this, the new type map becomes ["H", "O"].

This is not correct. The type map should be the union of two models.

@anyangml
Copy link
Collaborator

repeatedly each time it is fine-tuned

Indeed, I don't understand why it needs to change the type map each time it is fine-tuned...

For the LinearModel, let's say we have two pre-trained models: model A with ["H", "O", "Na"], model B with ["H", "O", "K"]. Now if we want to finetune a LinearModel with this, the new type map becomes ["H", "O"].

This is not correct. The type map should be the union of two models.

I think the combined model should only handle the common types. Suppose the new type map is the union of the two, there will be unseen types for each individual models.

@njzjz
Copy link
Member

njzjz commented Mar 14, 2024

repeatedly each time it is fine-tuned

Indeed, I don't understand why it needs to change the type map each time it is fine-tuned...

For the LinearModel, let's say we have two pre-trained models: model A with ["H", "O", "Na"], model B with ["H", "O", "K"]. Now if we want to finetune a LinearModel with this, the new type map becomes ["H", "O"].

This is not correct. The type map should be the union of two models.

I think the combined model should only handle the common types. Suppose the new type map is the union of the two, there will be unseen types for each individual models.

A model doesn't need to evaluate all types. A typical example is DPLR. Pairwise potentials may also be aimed at several certain types.

@wanghan-iapcm
Copy link
Collaborator

repeatedly each time it is fine-tuned

Indeed, I don't understand why it needs to change the type map each time it is fine-tuned...

because the user may provide new type_map that is not consistent with the model type_map

@njzjz njzjz added this to the v3.0.0 milestone Mar 16, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 22, 2024
This PR:
1. merge `change_energy_bias` into `compute_output_stats` and reformat
it into `change_out_bias` of `model` level.
2. support single-task/multi-task finetuning from single-task/multi-task
pretrained model.

Need fix in future PR:
1. Finetuned model has covered `type_map`. (If fixed, `change_out_bias`
func will not need the input params `origin_type_map` and
`full_type_map`.) See also #3455.
2. `change_out_bias` support for other models.(e.g. Spin, ZBL, Polar,
Dipole and Dos.)
@iProzd iProzd moved this to Backlog in DeePMD-3.0.0 beta release Apr 30, 2024
@njzjz njzjz linked a pull request Jun 6, 2024 that will close this issue
github-merge-queue bot pushed a commit that referenced this issue Jun 13, 2024
Fix #3747. Fix #3455. 

- Consistent fine-tuning with init-model, now in pt, fine-tuning include
three steps:
1. Change model params (for multitask fine-tuning, random fitting and
type-related params),
2. Init-model, 
3. Change bias

- By default, input will use user input while fine-tuning, instead of
being overwritten by that in the pre-trained model. When adding
“--use-pretrain-script”, user can use that in the pre-trained model.

- Now `type_map` will use that in the user input instead of overwritten
by that in the pre-trained model.

Note:
1. After discussed with @wanghan-iapcm, **behavior of fine-tuning in TF
is kept as before**. If needed in the future, it can be implemented
then.
2. Fine-tuning using DOSModel in PT need to be fixed. (an issue will be
opened, maybe fixed in another PR, cc @anyangml )

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

- **New Features**
- Added support for using model parameters from a pretrained model
script.
- Introduced new methods to handle type-related parameters and
fine-tuning configurations.

- **Documentation**
- Updated documentation to clarify the model section requirements and
the new `--use-pretrain-script` option for fine-tuning.

- **Refactor**
- Simplified and improved the readability of key functions related to
model training and fine-tuning.

- **Tests**
- Added new test methods and utility functions to ensure consistency of
type mapping and parameter updates.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Duo <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Han Wang <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@iProzd iProzd closed this as completed Jun 13, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in DeePMD-3.0.0 beta release Jun 13, 2024
mtaillefumier pushed a commit to mtaillefumier/deepmd-kit that referenced this issue Sep 18, 2024
Fix deepmodeling#3747. Fix deepmodeling#3455. 

- Consistent fine-tuning with init-model, now in pt, fine-tuning include
three steps:
1. Change model params (for multitask fine-tuning, random fitting and
type-related params),
2. Init-model, 
3. Change bias

- By default, input will use user input while fine-tuning, instead of
being overwritten by that in the pre-trained model. When adding
“--use-pretrain-script”, user can use that in the pre-trained model.

- Now `type_map` will use that in the user input instead of overwritten
by that in the pre-trained model.

Note:
1. After discussed with @wanghan-iapcm, **behavior of fine-tuning in TF
is kept as before**. If needed in the future, it can be implemented
then.
2. Fine-tuning using DOSModel in PT need to be fixed. (an issue will be
opened, maybe fixed in another PR, cc @anyangml )

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

- **New Features**
- Added support for using model parameters from a pretrained model
script.
- Introduced new methods to handle type-related parameters and
fine-tuning configurations.

- **Documentation**
- Updated documentation to clarify the model section requirements and
the new `--use-pretrain-script` option for fine-tuning.

- **Refactor**
- Simplified and improved the readability of key functions related to
model training and fine-tuning.

- **Tests**
- Added new test methods and utility functions to ensure consistency of
type mapping and parameter updates.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Duo <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Han Wang <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants