Skip to content

Conversation

@MGAMZ
Copy link
Contributor

@MGAMZ MGAMZ commented Oct 26, 2025

This is a sub-PR of #1665

Motivation

The torch.compile receives a lot args. And in mmengine, all contents in compile from any config file will be transferred to torch.compile.
The mmengine checks several dependencies when user specifies compile in their config file, and making hasattr(config, compile) as the flag of enabling torch.compile.
However this is not exact. The compile config can include an arg disable to state if the compiler is actually working. So the mmengine needs to carefully determine if the user is actually enabling the torch.compile. This caused a small modification in mmengine/_strategy/base.py

Modification

When user specified compile with a dict type, the compile_model func will check if disable is set to True, and can directly return the model just like compile is set to None

Copilot AI review requested due to automatic review settings October 26, 2025 07:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the logic for determining when torch.compile should be disabled by checking the disable flag within compile configuration dictionaries. Previously, the code only checked for boolean False values, missing cases where users explicitly set disable=True in their compile config.

Key changes:

  • Enhanced the early-return condition in compile_model() to handle the disable flag in dict-type compile configurations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

"""
if isinstance(compile, bool) and not compile:
if isinstance(compile, bool) and not compile or \
isinstance(compile, dict) and not compile.get('disable', False):
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

The condition logic is inverted. When disable=True, not compile.get('disable', False) evaluates to False, preventing the early return. The condition should be compile.get('disable', False) (without not) to return the model when disable is True.

Suggested change
isinstance(compile, dict) and not compile.get('disable', False):
isinstance(compile, dict) and compile.get('disable', False):

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@MGAMZ
Copy link
Contributor Author

MGAMZ commented Oct 26, 2025

@HAOCHENYE This one is ready to be reviewed.

@HAOCHENYE HAOCHENYE merged commit 30efd0c into open-mmlab:main Oct 26, 2025
5 of 7 checks passed
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.

2 participants