Skip to content

[CodeCamp2023-652] MMagic 新 config 体验与适配 StyleGAN3#2018

Merged
LeoXing1996 merged 9 commits intoopen-mmlab:mainfrom
hhy150:hhy150/CodeCamp2023_652_2
Sep 12, 2023
Merged

[CodeCamp2023-652] MMagic 新 config 体验与适配 StyleGAN3#2018
LeoXing1996 merged 9 commits intoopen-mmlab:mainfrom
hhy150:hhy150/CodeCamp2023_652_2

Conversation

@hhy150
Copy link
Contributor

@hhy150 hhy150 commented Sep 5, 2023

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

Please describe the motivation of this PR and the goal you want to achieve through this PR.

Modification

(1) Changed the name of the configuration file to comply with the Python module name specification, with only letters, numbers, and underscores included in the file name.
(2) Based on the new configs run through training, observe the dump configuration file, which is not significantly different from the old configs. As described in the tutorial, only the 'type' field is different, and the new configs file will contain the full information of the module.

  1. Verify the new and old configuration files.
    There are a total of 10 files:
    image
  2. Verify all configuration files after conversion
    image
    After verifying the script, there were no errors.

In addition, I also compared the configuration files obtained by running the training dump on the new and old configs, and found that they are only different in the "type" field. The new configs have full information. Some screenshots are as follows:
image

BC-breaking (Optional)

Does the modification introduce changes that break the backward-compatibility of the downstream repositories?
If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.

Use cases (Optional)

If this PR introduces a new feature, it is better to list some use cases here, and update the documentation.

Checklist

Submitting this pull request means that,

Before PR:

  • I have read and followed the workflow indicated in the CONTRIBUTING.md to create this PR.
  • Pre-commit or linting tools indicated in CONTRIBUTING.md are used to fix the potential lint issues.
  • Bug fixes are covered by unit tests, the case that causes the bug should be added in the unit tests.
  • New functionalities are covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  • The documentation has been modified accordingly, including docstring or example tutorials.

After PR:

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

@LeoXing1996
Copy link
Collaborator

Hey @hhy150, thanks for your contribution!
Could you please also upload a screenshot of the output of validation script and the generated result to ensure correctness of configs?

@hhy150
Copy link
Contributor Author

hhy150 commented Sep 5, 2023 via email

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Patch has no changes to coverable lines.

❗ Current head a354cd3 differs from pull request most recent head 82aa5aa. Consider uploading reports for the commit 82aa5aa to get more accurate results

Files Changed Coverage
mmagic/apis/inferencers/base_mmagic_inferencer.py 0.00%

📢 Thoughts on this report? Let us know!.

Copy link
Collaborator

@LeoXing1996 LeoXing1996 left a comment

Choose a reason for hiding this comment

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

LGTM

@LeoXing1996 LeoXing1996 merged commit e1bd41a into open-mmlab:main Sep 12, 2023
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.

3 participants