Skip to content

Add IR visitor to catch race conditions#121

Closed
jdahm wants to merge 15 commits intoGridTools:masterfrom
jdahm:validation-pass
Closed

Add IR visitor to catch race conditions#121
jdahm wants to merge 15 commits intoGridTools:masterfrom
jdahm:validation-pass

Conversation

@jdahm
Copy link
Copy Markdown
Contributor

@jdahm jdahm commented Jul 24, 2020

Implements node visitors to detect race conditions.

Resolves #130 [DSL-91].

Co-authors: @eddie-c-davis

@havogt havogt requested a review from gronerl July 24, 2020 16:20
Copy link
Copy Markdown
Contributor

@gronerl gronerl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some more cases that could be caught that aren't right now, such as in runtime conditionals and for k-PARALLEL cases.

Comment thread src/gt4py/frontend/gtscript_frontend.py Outdated
Comment thread src/gt4py/frontend/gtscript_frontend.py Outdated
Comment thread src/gt4py/frontend/gtscript_frontend.py Outdated
@jdahm jdahm marked this pull request as draft July 29, 2020 22:35
@jdahm jdahm changed the title Add passes that catch race conditions Add gtscript frontend visitor to catch race conditions Aug 3, 2020
@jdahm jdahm changed the title Add gtscript frontend visitor to catch race conditions Add IR visitor to catch race conditions Aug 4, 2020
@jdahm jdahm marked this pull request as ready for review August 4, 2020 18:10
@jdahm jdahm requested a review from gronerl August 4, 2020 19:29
InitInfoPass,
MergeBlocksPass,
NormalizeBlocksPass,
check_race_conditions,
Copy link
Copy Markdown
Contributor Author

@jdahm jdahm Aug 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be better put into an optimizations.py or ir_checkers.py ...

return cls()(root_node)

def __init__(self):
self.iteration_order = None
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused. Remove this before merging.

@gronerl
Copy link
Copy Markdown
Contributor

gronerl commented Aug 10, 2020

Hey, sorry i didn't reply sooner. According to the parallel model that we wrote down in the quickstart guide, this would actually not be the right behavior, namely the last test should exactly be valid. This sparked a discussion here about the parallel model which might change what is valid and what isn't. I assume and hope that networkx will not be necessary and that the tests can be done in the frontend. But this PR will remain pending until that is sorted out.

@twicki
Copy link
Copy Markdown
Contributor

twicki commented Aug 10, 2020

Are you referring to

For parallel regions, we can assume that each statement (i.e. each assign) is applied to the full domain, before the next one starts.

?

@jdahm
Copy link
Copy Markdown
Contributor Author

jdahm commented Aug 10, 2020

@gronerl I thought about a frontend check at first, but the code cannot easily tell what will create a race condition until the stencil definition is lowered to implementation IR and stages/multistages are created. Actually technically we probably could, but I don't have a handle on what conditions (e.g. run-time conditionals) force certain statements to stay part of a stage.

@jdahm
Copy link
Copy Markdown
Contributor Author

jdahm commented Aug 14, 2020

@eddie-c-davis and I worked on a version of this that works in the gtscript frontend, and raises an exception for the WAR issue: #143. I'll close this now in favor of that future PR.

@jdahm jdahm closed this Aug 14, 2020
@jdahm
Copy link
Copy Markdown
Contributor Author

jdahm commented Aug 14, 2020

See #148.

@jdahm jdahm deleted the validation-pass branch January 25, 2022 00:53
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 this pull request may close these issues.

Python backends compatibility with GT model

3 participants