Skip to content

Avoid usage of deprecated Indices class#2292

Merged
tbennun merged 8 commits intospcl:mainfrom
romanc:romanc/subsets-avoid-indices
Feb 6, 2026
Merged

Avoid usage of deprecated Indices class#2292
tbennun merged 8 commits intospcl:mainfrom
romanc:romanc/subsets-avoid-indices

Conversation

@romanc
Copy link
Contributor

@romanc romanc commented Feb 3, 2026

After a brief discussion subsets.Indices were deprecated last week with PR #2282. Since then, many Dace tests emit warnings because of remaining usage of Indices in the offset member functions of subsets.Range.

This PR suggests to adapt Range.from_indices() to add support for a sequence of (symbolic) numbers or strings (as suggested in Mattermost). This allows to remove the remaining usage of subsets.Indices constructors in the DaCe codebase, which gets rid of a bunch of warnings emitted in test or upstream/user code.

Only hickup that I had doing this was the function _add_read_slice() , called from visit_Subscript() of the ProgramVisitor in newast.py . That function would check for subsets to be either ranges or indices. And if subsets were indices, we'd go another way. That code path separation is apparently loosely tied to some other place in the codebase because we'd get errors if we were going the sub-optimal ranges-path with indices. I do now check if ranges are indices and set the flag accordingly. That seems to fix issues in tests.

I've also checked (manually) all other cases where we'd go a different code path in case subsets are indices. There are some and the remaining ones all "upgrade" indices to ranges. They can be removed once we remove the deprecated Indices class.

After a brief discussion `subsets.Indices` were deperecated last week with
PR spcl#2282. Since then, many Dace tests
emit warnings because of remaining usage of `Indices` in the offset
member functions of `subsets.Range`.

This PR suggests to adapt `Range.from_indices()` to add support for a
sequence of (symbolic) numbers or strings. This allows to remove the
remaining usage of `subsets.Indices` in the DaCe codebase and removes a
bunch of warnings emitted in test runs and/or when using the offset
member functions of `subset.Range`.
@romanc romanc marked this pull request as draft February 3, 2026 09:40
@romanc romanc changed the title refactor: avoid usage of depreated Indices class refactor: avoid usage of deprecated Indices class Feb 3, 2026
@romanc
Copy link
Contributor Author

romanc commented Feb 3, 2026

I've tracked down the last failure to simplify() in DaceProgram._parse(). Before is on the right and after simplify on the left. The first edge on the left assigns v = A[4].

image

The SDFG in the end translates to

void __program_copies_and_views_test_set_by_view_3_internal(copies_and_views_test_set_by_view_3_state_t*__state, int64_t * __restrict__ A)
{
    int64_t v;

    v = A[4];
    v = (v + 1);
}

which is why the test fails (because the write-back to A is now missing). Not sure at all where this comes from...

@romanc
Copy link
Contributor Author

romanc commented Feb 3, 2026

Update: I understand now, it's not simplify() to blame because the SDFG on the right already has no write-back to A anymore. The SDFG only writes back to v (which isn't tied to A in any way).

@romanc
Copy link
Contributor Author

romanc commented Feb 4, 2026

Update: this all comes back to _add_read_slice() which used Indices / Range as an indication of whether to make a view or not. Ranges get a view whereas indices are translated into a scalarized tmp access (as we see in the SDFGs above). With the changes so far, the access A[4] gets scalarized (because it's a one-element range) and thus no view is created and thus the write-back to A is missing.

In the parsed python this boils down to the difference between

# this is a view
v = A[4:5]
...

and

# this is a copy
v = A[4]
...

dace/subsets.py Outdated
@@ -1,10 +1,10 @@
# Copyright 2019-2025 ETH Zurich and the DaCe authors. All rights reserved.
from __future__ import annotations
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
from __future__ import annotations

We are already in the future

@tbennun tbennun self-requested a review February 6, 2026 09:44
Copy link
Collaborator

@tbennun tbennun left a comment

Choose a reason for hiding this comment

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

Fixed issue. Will merge if tests pass

@tbennun
Copy link
Collaborator

tbennun commented Feb 6, 2026

cscs-ci run

@tbennun tbennun marked this pull request as ready for review February 6, 2026 10:05
@tbennun tbennun changed the title refactor: avoid usage of deprecated Indices class Avoid usage of deprecated Indices class Feb 6, 2026
@tbennun tbennun added this pull request to the merge queue Feb 6, 2026
Merged via the queue into spcl:main with commit 1a88b3c Feb 6, 2026
12 checks passed
@tbennun tbennun deleted the romanc/subsets-avoid-indices branch February 6, 2026 16:17
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