Skip to content

Refactor push_to_hub #1883

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

Merged
merged 13 commits into from
Sep 2, 2024
Merged

Conversation

tastelikefeet
Copy link
Collaborator

PR type

  • Bug Fix
  • New Feature
  • Document Updates
  • More Models or Datasets Support

PR information

Write the detail information belongs to this PR.

Experiment results

Paste your experiment result here(if needed).

repo,
'configuration.json', ['{"framework": "pytorch", "task": "text-generation", "allow_remote": true}'],
ignore_push_error=True)
# Add '*.sagemaker' to .gitignore if using SageMaker
Copy link
Contributor

Choose a reason for hiding this comment

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

这个代码是从hf改的?怎么会想去特殊处理sagemaker环境。。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

是的,这里来自于之前的代码,处理sagemaker可能更好一些,毕竟影响ms和lf两个框架

logger = logging.get_logger(__name__)


class PushToMsHubMixin:
Copy link
Contributor

Choose a reason for hiding this comment

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

处理了ms和hf,是不是应该叫 PushToHubMixin 更通用一点

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

没有处理hf,hf直接跳过这里走原来的逻辑

oid=None,
)

if not _use_hf_hub:
Copy link
Contributor

Choose a reason for hiding this comment

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

为什么not use hf的时候处理了hf专门的逻辑,是写反了?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

没写反,这里直接hack底层的create/upload逻辑

hub_token = os.environ.get('MODELSCOPE_API_TOKEN')
if hub_token is not None:
api.login(hub_token)
visibility = ModelVisibility.PRIVATE if hub_private_repo else ModelVisibility.PUBLIC
Copy link
Contributor

Choose a reason for hiding this comment

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

else要报错吧。没有token是push不了的

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

user_name = ModelScopeConfig.get_user_info()[0]
assert isinstance(user_name, str)
hub_model_id = f'{user_name}/{hub_model_id}'
logger.info(f"'/' not in hub_model_id, setting hub_model_id: {hub_model_id}")
Copy link
Contributor

Choose a reason for hiding this comment

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

'/' not in hub_model_id, pushing to personal repo {hub_model_id}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

… feat/push_to_ms

* 'feat/push_to_ms' of github.com:tastelikefeet/swift:
  fix
@tastelikefeet tastelikefeet merged commit 543194c into modelscope:main Sep 2, 2024
2 checks passed
tastelikefeet added a commit to tastelikefeet/swift that referenced this pull request Sep 2, 2024
* main: (95 commits)
  support custom quantized dataset (modelscope#1893)
  Fix push_to_hub when last-checkpoint (modelscope#1897)
  bump version
  Add some warnings and fix RLHF (modelscope#1890)
  add vllm lmdeploy benchmark (modelscope#1889)
  Fix push to hub logic (modelscope#1888)
  Refactor push_to_hub (modelscope#1883)
  support qwen2-vl gptq awq (modelscope#1884)
  Support freeze_vit (modelscope#1880)
  use model.generation_config (modelscope#1850)
  add duet (modelscope#1877)
  fix doc (modelscope#1875)
  Fix num_proc (modelscope#1874)
  Add train record (modelscope#1873)
  [TorchAcc] fix serveral bugs for torchacc FSDP. (modelscope#1872)
  Support faster data map (modelscope#1871)
  update docs
  update docs qwen2-vl (modelscope#1869)
  update (modelscope#1864)
  update qwen2-vl docs (modelscope#1861)
  ...

# Conflicts:
#	swift/llm/sft.py
#	swift/llm/utils/template.py
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