Conversation
|
!test |
|
Review updated until commit aa333da Description
|
| Relevant files | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Enhancement | 6 files
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Documentation | 5 files
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Miscellaneous | 1 files
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Additional files | 36 files
|
PR Reviewer Guide
Here are some key observations to aid the review process:
| 🧪 PR contains tests |
| ⚡ Recommended focus areas for review |
Build System Logic Issue
|
Greptile OverviewGreptile SummaryThis PR removes the legacy Major Changes:
Critical Issues Found:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant PR as PR (Remove Legacy Bindings)
participant Build as Build System
participant Python as Python Environment
participant Scripts as Doc Scripts
Dev->>PR: Remove nvfuser module files
Dev->>PR: Remove python_frontend bindings
Dev->>PR: Remove csrc/serde/fusion_record
PR->>Build: Update CMakeLists.txt
Build-->>Build: Remove nvfuser._C extension
Build-->>Build: Move common sources to nvfuser_direct
Build-->>Build: Remove nvfuser symlink
PR->>Python: Update python/utils.py
Python-->>Python: Remove nvfuser._C from setup
PR->>Scripts: Add TODO comments
Note over Scripts: Scripts still import nvfuser module!
Scripts--xPython: import nvfuser (FAILS)
PR->>Build: Update tools/check_symbol_visibility.sh
Build-->>Build: Remove nvfuser extension checks
|
Additional Comments (2)
|
Additional Comments (1)
|
7d0a9ba to
904ab90
Compare
Additional Comments (7)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
21cb6bd to
5107f26
Compare
|
!test --dev |
|
|
||
| # TODO Update script to use nvfuser_direct module |
There was a problem hiding this comment.
TODO added but imports still reference removed nvfuser module - script will fail
| # TODO Update script to use nvfuser_direct module | |
| # TODO Update script to use nvfuser_direct module | |
| from nvfuser_direct import FusionDefinition, SchedulerType, DataType, ParallelType |
|
|
||
| # TODO Update script to use nvfuser_direct module |
There was a problem hiding this comment.
TODO added but imports still reference removed nvfuser module - script will fail
| # TODO Update script to use nvfuser_direct module | |
| # TODO Update script to use nvfuser_direct module | |
| from nvfuser_direct import FusionDefinition, SchedulerType |
| import math | ||
|
|
There was a problem hiding this comment.
TODO added but imports still reference removed nvfuser module - script will fail
| import math | |
| # TODO Update script to use nvfuser_direct module | |
| from nvfuser_direct import FusionDefinition, SchedulerType, DataType |
Additional Comments (3)
|
| import itertools | ||
|
|
||
| # TODO Update script to use nvfuser_direct module | ||
| from nvfuser import FusionDefinition, SchedulerType, DataType, ParallelType |
There was a problem hiding this comment.
Script imports from removed nvfuser module and will fail at runtime
| from nvfuser import FusionDefinition, SchedulerType, DataType, ParallelType | |
| from nvfuser_direct import FusionDefinition, SchedulerType, DataType, ParallelType |
| @@ -6,6 +6,8 @@ | |||
| import torch | |||
| import itertools | |||
| import math | |||
There was a problem hiding this comment.
Script imports from removed nvfuser module and will fail at runtime
Additional Comments (2)
|
This PR removes the
nvfuserpython module, corresponding pybind11 CPP bindings, and any references fromcsrc. Version is bumped to0.2.36.