ConditionalOptionalArrayResolver pass in preprocess

[FlorianDeconinck]

Introduce a new pass in preprocessing that aims to reduce the well define case of optional hinted arrays. E.g.

def program(array: Optional[]):
  if array is None:
    ...
  else:
    ...
    
program(None, ...)

See #1209 for full background

The PR adds a new pass and fix the optional flag on sdfg.arrays. Test test_optional_array_inference_via_parse in tests/python_frontend/optional_array_test.py

[FlorianDeconinck]

Fixing test it seem my original logic is flawed. Type hinting an array as Optional doesn't tell us anything more than "we should check at runtime". The way parsing is setup we skip looking at the actual given argument if any hinting is setup (e.g. argtypes build by _get_type_annotations, especially line 577)

So best we can do when we have a type hint is "maybe this will go away, we have to wait for call time to know". When no type hint is given, then we can infer because we are looking at given args, but isn't that more a side effect? Having the system behaves differently when type hinted or not seems dubious at best.

I can write the optional inference by actually resolving the given argument but that means making the SDFG dependent on the way it's called. E.g.

@dace.program
def meh( A: Optional[dace.Float64[2]]):
 if A is None:
  ...
 
 meh(A) <- one SDFG
 meh(None) <- another SDFG

Isn't that problematic in the wider usage of DaCe?

[alexnick83]

Fixing test it seem my original logic is flawed. Type hinting an array as Optional doesn't tell us anything more than "we should check at runtime". The way parsing is setup we skip looking at the actual given argument if any hinting is setup (e.g. argtypes build by _get_type_annotations, especially line 577)

So best we can do when we have a type hint is "maybe this will go away, we have to wait for call time to know". When no type hint is given, then we can infer because we are looking at given args, but isn't that more a side effect? Having the system behaves differently when type hinted or not seems dubious at best.

I can write the optional inference by actually resolving the given argument but that means making the SDFG dependent on the way it's called. E.g.

@dace.program
def meh( A: Optional[dace.Float64[2]]):
 if A is None:
  ...
 
 meh(A) <- one SDFG
 meh(None) <- another SDFG

Isn't that problematic in the wider usage of DaCe?

I think that this is fine. In fact, @tbennun isn't the purpose of the closure to make separate SDFGs only when the arguments change in a significant way?

[FlorianDeconinck]

Seems fair, I will write the code that go down that path and we can reason on it.

[tbennun]

Just make sure that if type hints were given, it’s AOT compilable

[FlorianDeconinck]

TL;DR: to make sure AOT based on type hints only is supported, no inference can be done, including in the OptionalArrayInference which might be a bug.

Going deeper on this, clearly the AOT & JIT clashes.

My understanding is as follows.

Given

@dace.program
def program(hinted_a: Optional[dace.float64[2]], b):
   if hinted_a is None:
      ...
   if b is None:
      ...

This should be compilable with

1. program.to_sdfg(A, B)
2. program.to_sdfg()

When down in parsing, I can't reason with given_args because of 2. Therefore I can only read the hint for the optional flag. This means that Optional=True is not an information we can infer on, since at call time we can have either an array or None in the case of 2 without triggering recompilation.

Happy to dive in this with you guys whenever you have time

[FlorianDeconinck]

The only option left is to restrict this pass to nested calls made with clear None type e.g.

def nested(A. B):
   ..

@dace.program
def program(A, B):
   nested(A, None) <- we can infer here when doing program.to_sdfg()

but we don't have the information in preprocess if this is the top-level or a nested call

[alexnick83]

The only option left is to restrict this pass to nested calls made with clear None type e.g.

def nested(A. B):
   ..

@dace.program
def program(A, B):
   nested(A, None) <- we can infer here when doing program.to_sdfg()

but we don't have the information in preprocess if this is the top-level or a nested call

The underlying issue is that we just don't support the redefinition of arrays and views (the latter is the issue in your case). An experimental SDFG feature supports redefinition, and I am considering using it to support redefining views as an experiment. However, I don't have an ETA because I will probably need to spend a day on that, and it may be hard to commit before the SC deadline. I will update you.

[FlorianDeconinck]

Agreed this would solve the original issue in Pace.

That said I believe the nested call version described above should still resolve at parse time. Now I don't have non-ugly way of writing it so it might a case of "when rewrite of frontend comes" as I also need to move forward and will have to use a workaround for now