Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deepcopying a nested SDFG does not copy constants #1375

Open
Sajohn-CH opened this issue Sep 27, 2023 · 2 comments · May be fixed by #1696
Open

Deepcopying a nested SDFG does not copy constants #1375

Sajohn-CH opened this issue Sep 27, 2023 · 2 comments · May be fixed by #1696

Comments

@Sajohn-CH
Copy link
Contributor

Describe the bug
If one deepcopy a SDFG of a nested SDFG where there are constants defined in the parent SDFG which are also present in the nested SDFG, they are not anymore after the deepcopy

To Reproduce
Execute the following code

import copy

import dace
from dace.memlet import Memlet


def main():
    sdfg = dace.SDFG('sdfg_copy_test')
    KLEV = dace.symbol('KLEV')
    sdfg.add_array('A', [KLEV], dace.float64)
    sdfg.add_array('B', [KLEV], dace.float64)
    state = sdfg.add_state()

    nsdfg_sdfg = dace.SDFG('sdfg_copy_test_nested')
    nsdfg_sdfg.add_array('A', [KLEV], dace.float64)
    nsdfg_sdfg.add_array('B', [KLEV], dace.float64)
    nsdfg_state = nsdfg_sdfg.add_state()

    nsdfg_state.add_mapped_tasklet(
        name="block_map",
        map_ranges={"JK": "0:KLEV"},
        inputs={"A": Memlet(data='A', subset='JK')},
        outputs={"B": Memlet(data='B', subset='JK')},
        code='b = a'
        )

    state.add_nested_sdfg(nsdfg_sdfg, sdfg, {'A'}, {'B'})

    # Either works
    # sdfg.specialize({'KLEV': 13})
    sdfg.add_constant('KLEV', 13)
    print(nsdfg_sdfg.constants)
    sdfg_copy = copy.deepcopy(nsdfg_sdfg)
    print(sdfg_copy.constants)


if __name__ == '__main__':
    main()

This will print

{'KLEV': 13}
{}

Expected behavior
Expect it to print:

{'KLEV': 13}
{'KLEV': 13}
@alexnick83
Copy link
Contributor

Constants are added to the (DaCe) property/dictionary SDFG.constants_prop. Constants are not copied to the nested SDFGs. Instead, the (Python) property SDFG.constants iterates recursively over the parent SDFG scopes to include their constants_prop items.

SDFG.__deepcopy__ does not copy an SDFG object exactly. Specifically, there is an issue with attributes such as SDFG.parent, SDFG.parent_sdfg, and SDFG.parent_nsdfg_node. These attributes reference objects (SDFGState, SDFG, NestedSDFG). If the SDFG being deep copied contains nested SDFGs, their (parent) reference attributes will be updated to point to the (deep) copied objects. However, it is a design decision that the (parent) reference attributes of the SDFG being copied are set to None. This is done because the deep copy method cannot know whether the SDFG copy will be placed in the same scope as the original or somewhere completely different. Therefore, it is left to the user to set those attributes explicitly.

However, the above design decision does not consider constants, as shown in the reported issue above. A potential issue from the user's perspective is that missing the original SDFG's parent constants may lead to some transformations failing to apply. For example, in CompositeFusion.can_be_applied, the SDFG has to be altered to complete all checks. Therefore, the SDFG is deep copied to avoid breaking the original. Should we update SDFG.constants_prop with SDFG.constants in such cases?

@tbennun
Copy link
Collaborator

tbennun commented Oct 29, 2024

will be resolved by #1696

@tbennun tbennun linked a pull request Oct 29, 2024 that will close this issue
6 tasks
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 a pull request may close this issue.

3 participants