Skip to content

Conversation

@BenkangPeng
Copy link
Contributor

Summary

This PR fixes a bug introduced during the TVM FFI refactoring where BaseComputeOp.axis,
BaseComputeOp.reduce_axis, and ScanOp.scan_axis attributes became inaccessible from Python.

Problem

After the TVM FFI library was separated to 3rdparty/tvm-ffi, the Python property wrappers
in python/tvm/te/tensor.py stopped working because they rely on __getattr__, which no
longer exists in the new FFI system.

Error when accessing these attributes:

import tvm
from tvm import te

A = te.placeholder((128, 128), name='A')
rk = te.reduce_axis((0, 128), name='k')
C = te.compute((128, 128), lambda i, j: te.sum(A[i, rk], axis=rk), name='C')

# This throws: AttributeError: 'ComputeOp' object has no attribute '__getattr__'
axis = C.op.axis

Root Cause

  1. In C++ (include/tvm/te/operation.h lines 147-149), BaseComputeOpNode's attributes are properly
    registered via FFI reflection:

    refl::ObjectDef<BaseComputeOpNode>()
        .def_ro("axis", &BaseComputeOpNode::axis)
        .def_ro("reduce_axis", &BaseComputeOpNode::reduce_axis);
  2. The new FFI system (3rdparty/tvm-ffi/python/tvm_ffi/registry.py) automatically
    creates Python properties for these C++ attributes.

  3. However, in python/tvm/te/tensor.py, the old @property wrappers using
    self.__getattr__("axis") block the FFI auto-generated properties from being added:

    # This prevents FFI from adding the auto-generated property
    @property
    def axis(self):
        return self.__getattr__("axis")  # __getattr__ doesn't exist anymore!
  4. The FFI registry skips adding properties if they already exist on the class

    ``3rdparty/tvm-ffi/python/tvm_ffi/registry.py` lines 272-275

    def _add_class_attrs(type_cls: type, type_info: TypeInfo) -> type:
        for field in type_info.fields:
            name = field.name
            if not hasattr(type_cls, name):  # skip already defined attributes
                setattr(type_cls, name, field.as_property(type_cls))

Solution

Remove the broken Python property wrappers in BaseComputeOp and ScanOp, allowing
the FFI system to automatically expose these C++ attributes as intended.

The FFI-generated properties provide the same functionality without requiring manual
wrapper code.

Changes

  • python/tvm/te/tensor.py:
    • Removed @property decorators for axis and reduce_axis in BaseComputeOp
    • Removed @property decorator for scan_axis in ScanOp.
    • Added docstring comments explaining that these attributes are auto-exposed by FFI

Testing

Verified that the attributes are now accessible:

import tvm
from tvm import te

n, k, m = 128, 128, 128
A = te.placeholder((n, k), name='A')
B = te.placeholder((k, m), name='B')
rk = te.reduce_axis((0, k), name='k')
C = te.compute((n, m), lambda i, j: te.sum(A[i, rk] * B[rk, j], axis=rk), name='C')

# These now work correctly
print(C.op.axis)          # [T.iter_var(i, T.Range(0, 128), "DataPar", ""), T.iter_var(j, T.Range(0, 128), "DataPar", "")]
print(C.op.reduce_axis)   # [T.iter_var(k, T.Range(0, 128), "CommReduce", "")]

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @BenkangPeng, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical issue introduced during the TVM FFI refactoring, which rendered several key properties (axis, reduce_axis, scan_axis) in BaseComputeOp and ScanOp inaccessible from Python. The fix involves removing redundant and now-broken Python property wrappers, thereby allowing the FFI system to properly expose these attributes as intended. This ensures that users can once again access these essential operational parameters, maintaining the integrity and usability of the TE module.

Highlights

  • Property Access Fix: Resolved a bug where BaseComputeOp.axis, BaseComputeOp.reduce_axis, and ScanOp.scan_axis properties were inaccessible from Python after recent TVM FFI refactoring.
  • Legacy Wrapper Removal: Removed outdated Python @property wrappers in BaseComputeOp and ScanOp that were using a non-existent __getattr__ method and preventing FFI from automatically exposing the C++ attributes.
  • FFI Integration: Enabled the new FFI system to correctly and automatically expose the C++ attributes for axis, reduce_axis, and scan_axis to Python, restoring intended functionality.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly fixes a bug where several attributes on BaseComputeOp and ScanOp were inaccessible from Python after an FFI refactoring. The fix involves removing the old, broken @property wrappers, which is the right approach. My review includes suggestions to add docstrings to the affected classes. This will improve code maintainability by explaining that these attributes are now automatically exposed by the FFI, a detail mentioned in the pull request description but missing from the code changes.

Comment on lines 131 to 132
class BaseComputeOp(Operation):
"""Compute operation."""
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

While removing the broken properties is correct, it would be beneficial for code readability and maintainability to add a docstring explaining that axis and reduce_axis are automatically exposed by the FFI, as you mentioned in the PR description. This helps future developers understand where these attributes come from without having to trace back to the C++ FFI registration.

How about updating the class docstring?

Suggested change
class BaseComputeOp(Operation):
"""Compute operation."""
class BaseComputeOp(Operation):
"""Compute operation.
The `axis` and `reduce_axis` attributes are automatically exposed by the FFI.
"""

Comment on lines 141 to 142
class ScanOp(Operation):
"""Scan operation."""
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Similar to BaseComputeOp, it would be helpful to add a docstring to ScanOp to clarify that the scan_axis attribute is exposed via the FFI. This improves discoverability and maintainability.

Suggested change
class ScanOp(Operation):
"""Scan operation."""
class ScanOp(Operation):
"""Scan operation.
The `scan_axis` attribute is automatically exposed by the FFI.
"""

@tqchen tqchen merged commit 2fd72ab into apache:main Oct 16, 2025
14 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