Fix: error with slice when using descript_scheduler by yadej · Pull Request #27 · xtc-tools/xtc

yadej · 2026-01-27T09:27:24Z

Add a minimal failing example for descript_scheduler

Discussion of the error is in that issue #26

qaco · 2026-01-28T09:33:34Z

Cool ! Do you want to dig into the bug (probably in descript.py) and fix it in the same PR ? It could be a good exercise to discover XTC deeper. If you prefer, I can do it.

yadej · 2026-01-28T09:40:18Z

I want to try to fix it

qaco · 2026-01-30T23:04:06Z

src/xtc/schedules/descript.py

+            new_interchange = []
+            for loop_name in sched.interchange:
+                if not loop_name in sched.splits_to_sizes:
+                    loop_name = re.sub(r"\[.*?\]$", "", loop_name)


Here you recover the abstract axis from the split declaration using the naming convention, am I right ? Why is it not in splits_to_sizes ?

When we create a inner_nest from the split the head of the interchange will be [i[0]] and [i[1]] for the 2 split and if we use splits_to_sizes on those slice there will be no i[0] or i[1] since it is only in the first splits_to_sizes slice

Actually, you've put your finger on a misconception that I introduced (see my comment on my PR). Thank you !

In fact, I added these heads because in MLIR, the split cuts the linalg operator but doesn't materialize the loops, so this head corrects the behavior. But this should be handled at the MLIR backend level, not at the abstract/descript schedule level. At this point, I think we need to remove this head mechanism which is really error-prone.

Now that part is done on mlir backend instead of descript

qaco · 2026-01-30T23:07:51Z

src/xtc/schedules/descript.py

        self._check_tiling_consistency()
        self._check_sizes()

+    def remove_excess_interchange(self):


If I understand well, the point here is to express "i, j[X], k" as "i, j, k" ?

that's right

Once they are created, we should not change the names, because they identify uniquely a loop. In this case, the problem is deeper : this loop should not exist (see my other comment).

It is now removed

yadej · 2026-02-03T12:44:57Z

src/xtc/backends/mlir/MlirCompilerPasses.py

                continue
+            loop_name_no_root = loop_name.rsplit("/", 1)[-1]
+            loop_name_no_root = loop_name_no_root.split("[")[0]
+            loop_name = root + "/" + loop_name_no_root


Do I keep that to have the node name such as __node0__/J[0]/J or do I change it to __node0__/J[0]/J[0] but I will need to change most mlir splitting test

No __node0__/J[0]/J is fine. But you need to build the name from the root + abstract axis, not from the split name (we want this algorithm to work even if name conventions change).

Besides could you please use ROOT_SEP instead of literal '/' ?

I replaced the '/' with ROOT_SEP and put that in itf.schd.scheduler and added a constant SPLIT_LEFT_SEP for "[" and SPLIT_RIGHT_SEP for "]"

I found a way to build the name with root and abstract axis without passing via string splitting

qaco · 2026-02-03T23:32:27Z

src/xtc/schedules/descript.py

-                f"Axis {axis_name} is scheduled twice (or more)."
-            )
+        for loop_name in interchange:
+            loop_name = re.sub(r"\[.*?\]$", "", loop_name)


Same remark as above. We don't want to rely on naming conventions.

qaco · 2026-02-03T23:33:58Z

src/xtc/schedules/descript.py


        # Recursively interpret the nested schedule
-        inner_nest = self._interpret_spec(item.body, new_root_name, head=[axis_name])
+        inner_nest = self._interpret_spec(item.body, new_root_name, head=[new_dim_name])


It seems to me that the parameter head is now useless. Do you agree ?

Yes now the parameter head is removed

descript_scheduler - Remove test slice file bug since its fix - Add test to descript_scheduler with slice

in _interpret_spec in descript - Remove head from _interpret_spec in descript - Add Constant name SPLIT_LEFT_SEP for [ and SPLIT_RIGHT_SEP for ]

guillon added the enhancement New feature or request label Jan 28, 2026

guillon requested a review from qaco January 28, 2026 09:26

guillon assigned qaco Jan 28, 2026

guillon assigned yadej Jan 29, 2026

yadej force-pushed the dev/rcesista/spec_schedule_bug branch from ebd6146 to 98b2aff Compare January 29, 2026 15:52

yadej changed the title ~~Add test file for error with slice when using descript_scheduler~~ Fix: error with slice when using descript_scheduler Jan 29, 2026

qaco reviewed Jan 30, 2026

View reviewed changes

yadej force-pushed the dev/rcesista/spec_schedule_bug branch from 98b2aff to 3ab1ee6 Compare February 3, 2026 12:36

yadej commented Feb 3, 2026

View reviewed changes

yadej requested a review from qaco February 3, 2026 12:51

qaco reviewed Feb 3, 2026

View reviewed changes

yadej added 9 commits February 20, 2026 11:28

Add test file for error with slice when using descript_scheduler

6559566

Fix: Splitting with a smaller size than the next slice with

1490b05

descript_scheduler - Remove test slice file bug since its fix - Add test to descript_scheduler with slice

Move the ScheduleError of remove_excess to more appropriate place

da41747

Remove remove_excess that change interchange in descript

9122492

fix tests with the new interchange

c7d3f31

Remove useless if statement

2b612c6

Add and move CONSTANT into itf.schd.scheduler and remove unecessary arg

2d484fa

in _interpret_spec in descript - Remove head from _interpret_spec in descript - Add Constant name SPLIT_LEFT_SEP for [ and SPLIT_RIGHT_SEP for ]

Add None check in Descript check sizes

70c0d55

Use abstract axis instead of passing via string splitting

97e5311

yadej force-pushed the dev/rcesista/spec_schedule_bug branch from 0120f52 to 97e5311 Compare February 20, 2026 10:28

yadej requested a review from qaco February 20, 2026 10:38

Comments

Conversation

yadej commented Jan 27, 2026

Uh oh!

qaco commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yadej commented Jan 28, 2026

Uh oh!

qaco Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qaco Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qaco commented Jan 28, 2026 •

edited

Loading

qaco Jan 30, 2026 •

edited

Loading

qaco Feb 2, 2026 •

edited

Loading