spcl / dace

DaCe - Data Centric Parallel Programming
http://dace.is/fast
BSD 3-Clause "New" or "Revised" License
497 stars 129 forks source link

Deepcopying a nested SDFG does not copy constants #1375

Open Sajohn-CH opened 1 year ago

Sajohn-CH commented 1 year ago

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 commented 1 year ago

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 commented 2 weeks ago

will be resolved by #1696