numba / numba-rvsdg

Numba compatible RVSDG (Regionalized Value State Dependence Graph) utilities.
https://numba-rvsdg.readthedocs.io/
BSD 2-Clause "Simplified" License
18 stars 7 forks source link

Test rendering #57

Closed esc closed 6 months ago

esc commented 1 year ago

This PR stabilizes rendering. There are a number of calls to sorted both within the algorithms and also within the rendering code that were needed to make this happen. The non-deterministic behaviour is almost always the result of iterating over a set somehere. Many of the tests don't show this, because they just compare the graph structurally. However, comparing dot syntax, requires all the node names and their order to be predictable.

I will comment on each call to sorted to discuss where this comes from and why it is needed.

esc commented 1 year ago

I commented on the calls to sorted -- it is mostly due to ambiguities over the order in which synthetic assignment blocks are inserted into the graph, and many issues caused by initializing subgraphs based on iterating over a set.

An additional test (finally) for fig 3. is also included. And we have the first, very basic region test, by virtue of testing the rendering.

esc commented 1 year ago

I can't seem to replicate the error I am seeing on CI, it is:

FAILED numba_rvsdg/tests/test_fig3.py::test_fig3 - AssertionError: assert 'digraph {\n\...ad_block_0\n}' == 'digraph {\n\..._block_0\n}\n'
  Skipping 4472 identical leading characters in diff, use -v to show
    d_block_0
  - }
  + }
FAILED numba_rvsdg/tests/test_rendering.py::test_simple - AssertionError: assert 'digraph {\n\...ad_block_0\n}' == 'digraph {\n\..._block_0\n}\n'
  Skipping 2740 identical leading characters in diff, use -v to show
    d_block_0
  - }
  + }
esc commented 1 year ago

I found a solution to the bug on CI -- seems like it was the result of whitespace. A strip on the Graphviz output makes this PR pass.

I added a test for fig. 4 also.

esc commented 6 months ago

replaced by #118