Avoid usage of deprecated state.add_array() and state.add_stream()
#2298
Conversation
tbennun
changed the title
state.add_array() and `state.add_stream()state.add_array() and state.add_stream()
romanc
force-pushed
the
romanc/avoid-state-add_array
branch
from
e043cd4 to
9808e84
Compare
tbennun
changed the title
state.add_array() and state.add_stream()state.add_array() and state.add_stream()
tbennun
left a comment
tbennun
left a comment
There was a problem hiding this comment.
Minor change but otherwise LGTM!
dace/sdfg/utils.py
Outdated
| # Loads compiled SDFG. | ||
| if rank > 0: | ||
| func = load_precompiled_sdfg(folder) | ||
| func = load_precompiled_sdfg(folder, argnames=argnames) |
There was a problem hiding this comment.
I don't understand this. Shouldn't sdfg.argnames exist if argnames was not provided?
There was a problem hiding this comment.
Yes, that exists. My reading of
dace/dace/codegen/compiled_sdfg.py
Lines 254 to 256 in 90d45d9
is that relying on this information is deprecated. I guess I don't understand correctly. Of course, I could provide argnames from sdfg.argnames inside load_precompiled_sdfg and then non of these changes are necessary, but that feels kind of like defeating the purpose, no? Unless, that was the plan all along and PR https://github.com/spcl/dace/pull/2291/changes shouldn't have added argnames as optional argument to load_precompiled_sdfg in the first place.
There was a problem hiding this comment.
As you can see in the initial commit that triggered this warning, the correct way to deal with this argument is to pass in the arg_names value if no one else overrode it: 5248dad
There was a problem hiding this comment.
ooh, so there's arg_names and argnames. Now I see. I'll fix it up tomorrow. Thanks for the pointer!
tests/library/mpi/mpi4py_test.py
Outdated
| if rank == 0: | ||
| sdfg = comm_world_bcast.to_sdfg() | ||
| func = utils.distributed_compile(sdfg, commworld) | ||
| func = utils.distributed_compile(sdfg, commworld, argnames=["A"]) |
There was a problem hiding this comment.
I believe all changes here will not be necessary if my previous comment is addressed
| if rank == 0: | ||
| sdfg = mpi4py_isend_irecv.to_sdfg(simplify=True) | ||
| func = utils.distributed_compile(sdfg, commworld) | ||
| func = utils.distributed_compile(sdfg, commworld, argnames=["rank", "size"]) |
| # Disable OpenMP section to allow blocking | ||
| sdfg.openmp_sections = False | ||
| mpi_sdfg = utils.distributed_compile(sdfg, comm) | ||
| mpi_sdfg = utils.distributed_compile(sdfg, comm, argnames=["rank", "size"]) |
| raise ValueError("Please run this test with at least two processes.") | ||
|
|
||
| func = utils.distributed_compile(sdfg, commworld) | ||
| func = utils.distributed_compile(sdfg, commworld, argnames=["dims", "periods", "coords", "valid", "P"]) |
| if rank == 0: | ||
| sdfg = matrix_2d_2d.to_sdfg() | ||
| func = utils.distributed_compile(sdfg, commworld) | ||
| func = utils.distributed_compile(sdfg, commworld, argnames=["A"]) |
tests/library/mpi/subarrays_test.py
Outdated
| func = utils.distributed_compile(sdfg, commworld, argnames=["A"]) | ||
|
|
tests/chained_nested_tasklet_test.py
Outdated
| A_ = state.add_access("A") | ||
| B_ = state.add_access("B") |
There was a problem hiding this comment.
| A_ = state.add_access("A") | |
| B_ = state.add_access("B") | |
| A_ = state.add_access('A') | |
| B_ = state.add_access('B') |
romanc
force-pushed
the
romanc/avoid-state-add_array
branch
from
f308da8 to
9f4dfaa
Compare
romanc
force-pushed
the
romanc/avoid-state-add_array
branch
from
3135e9a to
592ffc1
Compare
2 participants
Get the number of warnings in tests down by avoiding usage of
state.add_*functions likestate.add_array(...). Also fixes deprecation warnings in mpi tests by passing argnames to distributed compile.Based on top of #2299.