GTIR domain inference #1568
GTIR domain inference #1568
Conversation
tests/next_tests/unit_tests/iterator_tests/transforms_tests/test_domain_inference.py
Outdated
Show resolved
Hide resolved
|
Note to myself: One function for itir.Program with temporaries, one without (only the simple for loop). When writing temporary pass introduce |
…add functionality to infer domain of a program with several SetAt
This PR fixes a bug where the size of unstructured temporary fields was wrong in certain cases. This bug surfaced while working on #1568. It took me a while to figure out why this never surfaced in validation tests anywhere: Right now we compute temporaries for unstructured everywhere, e.g. on all vertices, etc.. What _everywhere_ means is derived from the offset provider in `_max_domain_sizes_by_location_type`. The information is contained either - explicitly if we have a connectivity with origin axis being the axis whose size we want to know. E.g. if we have a V2E connectivity then we have `table.shape[0]` many vertices. - or implicitly if we have a connectivity with the neighbor axis whose size we want to know. E.g. if we have a V2E connectivity then we have at least `table.max()+1` many edges. This computation was wrong. So as long as we have also the E2V connectivity for the examples above, the implicit information is not needed / used and everything works fine. For all our test cases this is indeed the case and since computing temporaries everywhere is only a temporary solution anyway I am fine with just fixing this without increasing test coverage. --------- Co-authored-by: edopao <edoardo.paone@cscs.ch>
…iterals to as_fieldop
…IR_domain-inference
| transformed_set_ats.insert( | ||
| 0, | ||
| itir.SetAt( | ||
| expr=transformed_as_fieldop, | ||
| domain=SymbolicDomain.as_expr(accessed_domains[set_at.target.id]), | ||
| target=set_at.target, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
updating the domain should only be done if the original domain is AUTO_DOMAIN, correct? I am wondering if we should split this pass in 2. One that propagates domains between set_at (i.e. resolves AUTO_DOMAIN) and one that just annotates as_fieldops. I find it confusing that we do both in the same pass, but not sure if I am missing something.
There was a problem hiding this comment.
I am also not sure if I am missing something, but I think this is only possible with some code duplication.
One option could be to add a pass that infers the domains of all as_fieldops which is then only called by DaCe.
Let's discuss this next time we talk.
There was a problem hiding this comment.
As discussed: We discovered that running the domain propagation prior to temporary extraction should make all temporary related code in the propagation unnecessary and can hence be removed. We've placed a todo (in the let branch) and kept it in here for now so that we can move on.
) Infers the minimal domain of (nested) `let`, `make_tuple`, `tuple_get`, `cond` and other builtins as an extension to PR #1568 - New functions `infer_let`, `infer_make_tuple`, `infer_tuple_get`, `infer_cond` in `gt4py.next.iterator.transforms.infer_domain` - New function `infer_expr` in gt4py.next.iterator.transforms.infer_domain which calls the appropriate of the above (or `infer_as_fieldop` and `infer_program`) - Several new tests in test_infer_domain.py to test functionality Note: Temporary handling was only present until commit fc4846f and has been removed in commit e8e679d to reduce unneeded complexity. This pass will be executed before temporary extraction, hence there exist valid `domain`s in all program calls, i.e. all `SetAt` do have a domain (not `AUTO_DOMAIN`) that doesn't need to be inferred. --------- Co-authored-by: Till Ehrengruber <till.ehrengruber@cscs.ch>
Uses TraceShifts to infer the minimal domain of (nested)
as_fieldops and SetAt:Domain inference of let-statements is not implemented yet and will be done in PR #1591
PR Description
infer_as_fieldopandinfer_programdomainandas_fieldopin ir.makersitir.Literals to simplify resulting domainsTodos
as_fieldops (cartesian_domain)Note
I agree that my contribution in this PR is additionally licensed under the BSD-3-Clause license (SPDX short identifier).