Add Docker support and update test/demo scripts to use automatic Hugging Face downloads#28
Add Docker support and update test/demo scripts to use automatic Hugging Face downloads#28
Conversation
Introduced a Makefile to streamline Docker image building and container management. Added targets for building, running, and executing commands within the Docker container, as well as a production build process. Included help instructions for usage.
Created a Dockerfile to establish a development environment for FMPose3D, including installation of necessary packages, user setup, and configuration for Oh My Zsh. The Dockerfile also ensures proper permissions for model weights and initializes conda for the user.
…_video_kpts function
Modified the script to enable automatic downloading of model weights from Hugging Face Hub by default. Added a commented line for users to specify local weights if desired.
Modified the script to set model weights to an empty string by default, enabling automatic downloading from Hugging Face Hub. Added a commented line for users to specify local weights if needed.
…ic downloading from Hugging Face Hub. Added logic to download weights if no local path is specified, improving user experience and flexibility.
…ights from Hugging Face Hub. Updated logic to handle cases where no local weights path is specified, improving usability and flexibility for users.
There was a problem hiding this comment.
Pull request overview
Adds a Docker-based development environment and updates demo/test entrypoints to default to automatic model-weight downloads from Hugging Face when no local weights path is provided, while deferring HRNet checkpoint downloads until HRNet is actually invoked.
Changes:
- Introduces a CUDA-enabled Dockerfile and a Makefile workflow for building/running dev containers.
- Updates
scripts/anddemo/Python + shell entrypoints to auto-download*.pthweights from Hugging Face whenmodel_weights_pathis empty. - Adjusts HRNet keypoint generation to download checkpoints only when
gen_video_kpts()is called.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Dockerfile |
Adds a PyTorch CUDA runtime image with dev shell tooling and non-root user setup. |
Makefile |
Adds Docker build/run/exec targets and a “production” build target. |
scripts/FMPose3D_main.py |
Adds HF auto-download logic and adjusts checkpoint output directory handling when weights are not local. |
scripts/FMPose3D_test.sh |
Defaults model_weights_path to empty to trigger HF downloads. |
demo/vis_in_the_wild.py |
Adds HF auto-download logic for demo weights loading. |
demo/vis_in_the_wild.sh |
Defaults model_weights_path to empty to trigger HF downloads. |
fmpose3d/lib/hrnet/gen_kpts.py |
Defers ensure_checkpoints() until HRNet keypoint generation is requested. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
scripts/FMPose3D_main.py
Outdated
| # If no local path is provided, download from Hugging Face Hub | ||
| if not model_path: | ||
| from huggingface_hub import hf_hub_download | ||
| _HF_REPO_ID = "deruyter92/fmpose_temp" | ||
| hf_filename = f"{args.model_type}.pth" | ||
| print(f"No local weights path specified. Downloading '{hf_filename}' from Hugging Face ({_HF_REPO_ID})...") | ||
| model_path = hf_hub_download(repo_id=_HF_REPO_ID, filename=hf_filename) |
demo/vis_in_the_wild.py
Outdated
| # If no local path is provided, download from Hugging Face Hub | ||
| if not model_path: | ||
| from huggingface_hub import hf_hub_download | ||
| _HF_REPO_ID = "deruyter92/fmpose_temp" | ||
| hf_filename = f"{args.model_type}.pth" | ||
| print(f"No local weights path specified. Downloading '{hf_filename}' from Hugging Face ({_HF_REPO_ID})...") | ||
| model_path = hf_hub_download(repo_id=_HF_REPO_ID, filename=hf_filename) | ||
|
|
There was a problem hiding this comment.
Same as in scripts/FMPose3D_main.py: the Hugging Face repo ID + download logic is duplicated and hard-coded here. Reusing a shared helper/constant (e.g., from the inference API module) would reduce drift and keep the demo aligned with the packaged API behavior.
fmpose3d/lib/hrnet/gen_kpts.py
Outdated
| @@ -153,6 +152,9 @@ def gen_from_image(args, frame, people_sort, human_model, pose_model, det_dim=41 | |||
|
|
|||
|
|
|||
| def gen_video_kpts(path, det_dim=416, num_peroson=1, gen_output=False, type='image'): | |||
There was a problem hiding this comment.
Typo in the public function signature: num_peroson should be num_person. Since this function is imported/used elsewhere, rename the parameter and update call sites to avoid perpetuating the misspelling.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Fixed in commit 6254cc8. Renamed num_peroson → num_person in gen_from_image and gen_video_kpts signatures in gen_kpts.py, and updated all call sites in demo/vis_in_the_wild.py and tests/test_demo_human.py.
…e_weights_path function. This change simplifies the logic for loading model weights, enhancing usability by automatically handling local and remote paths.
…weights_path function. This change simplifies the logic for loading model weights, improving usability by automatically handling local and remote paths.
…is change centralizes the repository ID definition, improving maintainability and consistency across the codebase.
Introduced a new utility function in weights.py to handle the resolution of model weights paths. This function allows users to specify a local path or automatically download weights from the Hugging Face Hub if no local path is provided, enhancing usability and flexibility in model weight management.
This new file serves as an initializer for the utils module, allowing for better organization and potential future utility functions related to FMPose3D.
…ng dependencies Modified the test to raise an ImportError with a specific message when the required 'fmpose3d[animals]' package is not installed, improving error handling and clarity in dependency management.
scripts/FMPose3D_main.py
Outdated
| # If no local path is provided, download from Hugging Face Hub | ||
| if not model_path: | ||
| from huggingface_hub import hf_hub_download | ||
| _HF_REPO_ID = "deruyter92/fmpose_temp" | ||
| hf_filename = f"{args.model_type}.pth" | ||
| print(f"No local weights path specified. Downloading '{hf_filename}' from Hugging Face ({_HF_REPO_ID})...") | ||
| model_path = hf_hub_download(repo_id=_HF_REPO_ID, filename=hf_filename) |
Agent-Logs-Url: https://github.com/AdaptiveMotorControlLab/FMPose3D/sessions/2136af56-29a8-4635-9ed4-faf9b8146672 Co-authored-by: xiu-cs <81274389+xiu-cs@users.noreply.github.com>
…accordingly. This change simplifies the build process by eliminating the need for a user-defined source path.
…" to "matrices" and "projectio" to "projection". Additionally, update comment for testing section to reflect correct function name "triangulate".
…lining the inference API code. This change reflects the transition towards a more permanent solution for weight management.
…tes" for improved clarity.
…ion" for improved clarity.
…rguements" to "arguments" and "availible" to "available".
…eamlining the code and improving clarity.
…o "original" and "parameteres" to "parameters".
…mproved clarity in keypoint normalization.
…o "original" and "parameteres" to "parameters".
…itys.py to streamline the code and improve maintainability.
…test organization
…hancing installation process by cloning the repository and cleaning up afterwards.
…omatic downloads from Hugging Face and providing alternative options for local weights.
…onstant `HF_REPO_ID`)
deruyter92
left a comment
There was a problem hiding this comment.
Looks great, very useful PR!
- The typo fixes look all good to me!
- Nice work on centralizing the HugginFace path loading.
- Regarding the docker, you might want to consider improving the reproducibility by pinning the version and making it more robust for all platforms. See my comments.
- Thanks for your fixes in the inference api as well! I pushed a small additional change to test_huggingface.py, making use of your new centralized HF_REPO_ID constant.
demo/vis_in_the_wild.py
Outdated
| model_dict = model['CFM'].state_dict() | ||
| model_path = args.model_weights_path | ||
| print(model_path) | ||
| from fmpose3d.utils.weights import resolve_weights_path |
There was a problem hiding this comment.
Probably better to move to the top, unless you intentionally want it to load only locally in this function ('lazy loading'). But since it is a lighweight import I don't think it provides a real benefit to keep it here.
| RUN python -m pip install --no-cache-dir --upgrade pip && \ | ||
| git clone --depth 1 https://github.com/AdaptiveMotorControlLab/FMPose3D.git /tmp/fmpose3d && \ | ||
| python -m pip install --no-cache-dir "/tmp/fmpose3d[animals,viz]" gdown && \ | ||
| rm -rf /tmp/fmpose3d |
There was a problem hiding this comment.
Since docker is often used for reproducability, I think it would be best if you pin a specific version here, instead of cloning at HEAD.
There was a problem hiding this comment.
Good suggestions! Actually, for the first version, I pinned a specific version, but this PR changes some functions inside the package, which makes the currently released package version not work with the training code.
| RUN groupadd ${USERNAME} --gid ${USER_GID} | ||
| RUN useradd -m -s /bin/bash -g ${USERNAME} -u ${USER_UID} ${USERNAME} |
There was a problem hiding this comment.
Just to be safe, you could change this to the below suggestion, since the current command may fail if the user allready exists (e.g. in the parent image). But I don't expect that this is a realistic scenario, so you can also keep yours, for better readability.
| RUN groupadd ${USERNAME} --gid ${USER_GID} | |
| RUN useradd -m -s /bin/bash -g ${USERNAME} -u ${USER_UID} ${USERNAME} | |
| RUN set -eux; \ | |
| if getent group "${USER_GID}" >/dev/null; then \ | |
| GROUP_NAME="$(getent group "${USER_GID}" | cut -d: -f1)"; \ | |
| else \ | |
| groupadd --gid "${USER_GID}" "${USERNAME}"; \ | |
| GROUP_NAME="${USERNAME}"; \ | |
| fi; \ | |
| if id -u "${USER_UID}" >/dev/null 2>&1; then \ | |
| EXISTING_USER="$(getent passwd "${USER_UID}" | cut -d: -f1)"; \ | |
| usermod -l "${USERNAME}" "${EXISTING_USER}" || true; \ | |
| usermod -d "/home/${USERNAME}" -m "${USERNAME}" || true; \ | |
| usermod -g "${GROUP_NAME}" "${USERNAME}" || true; \ | |
| else \ | |
| useradd -m -s /bin/bash -u "${USER_UID}" -g "${GROUP_NAME}" "${USERNAME}"; \ | |
| fi |
|
|
||
| #: HuggingFace repository hosting the official FMPose3D checkpoints. | ||
| _HF_REPO_ID: str = "deruyter92/fmpose_temp" | ||
| from fmpose3d.utils.weights import HF_REPO_ID as _HF_REPO_ID |
There was a problem hiding this comment.
Good fix, thanks! Maybe just move it to the import block with the other from fmpose3d ... imports.
| @echo " 3. Inside container: pip install -e '.[animals,viz]'" | ||
| @echo " 4. Inside container: sh scripts/FMPose3D_train.sh" |
There was a problem hiding this comment.
the dockerfile allready installs fmpose, right?
| @echo " 3. Inside container: pip install -e '.[animals,viz]'" | |
| @echo " 4. Inside container: sh scripts/FMPose3D_train.sh" | |
| @echo " 3. Inside container: sh scripts/FMPose3D_train.sh" |
scripts/FMPose3D_main.py
Outdated
| model_dict = model["CFM"].state_dict() | ||
| model_path = args.model_weights_path | ||
| print(model_path) | ||
| from fmpose3d.utils.weights import resolve_weights_path |
There was a problem hiding this comment.
consider moving this to the top.
|
side note: maybe not for this PR, but in the future, you may consider setting up linting for automating the formatting a bit. |
…ts_path and add it back in a more appropriate location for better code organization.
Co-authored-by: Jaap de Ruyter van Steveninck <32810691+deruyter92@users.noreply.github.com>
Co-authored-by: Jaap de Ruyter van Steveninck <32810691+deruyter92@users.noreply.github.com>
Co-authored-by: Jaap de Ruyter van Steveninck <32810691+deruyter92@users.noreply.github.com>
Co-authored-by: Jaap de Ruyter van Steveninck <32810691+deruyter92@users.noreply.github.com>
… top for better organization and remove redundant import in the main execution block.
This pull request introduces a Docker-based development environment for FMPose3D, adds a Makefile for common Docker workflows, and improves the robustness and maintainability of the codebase. It also fixes several typos and cleans up unused code. The most important changes are summarized below.
Dockerization and Development Workflow:
Dockerfileto provide a reproducible environment using PyTorch with CUDA, pre-installs FMPose3D and its dependencies, sets up a non-root user, and configures shell environments for development.Makefilewith targets for building, running, and managing the Docker container, streamlining local development and GPU-enabled workflows.Dependency Management and Weights Handling:
fmpose3d.utils.weights. [1] [2]Bug Fixes and Code Quality:
num_peroson→num_person,hight→height) and comments throughout the codebase for clarity and correctness. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]plot_keypoint,write, andload_jsonfunctions fromutilitys.py, and the temporary weights validation skip logic. [1] [2] [3] [4]These changes collectively make the project easier to set up and contribute to, improve code maintainability, and reduce sources of error.