feat: add observer-based recording and animation for the canonicalizer#207
Conversation
|
cc @AmanieOxana |
|
There are lint errors in the code, please look at them and correct them |
|
Hey @gksmail now thw ci should be green! |
Hi, can you change the title of the last frame? Add the algebra name to the title. |
|
|
||
| .. table:: Colour legend for the node roles tracked while building the canonical graph. | ||
|
|
||
| +-----------------------+--------------------------------------------------------------+ |
There was a problem hiding this comment.
People cannot associate uncommon colours with names easily. I would suggest placing a box showing the colour next to the name or something similar, and/or using more distinguishable colours.
| algebra = 4*so(5) | ||
|
|
||
| .. Here we should add some definitions like lit, dependent etc. | ||
| The animation below follows the construction of one of the four canonical components. Starting from |
There was a problem hiding this comment.
The animations as they are right now change very fast. Previously, animations had a control bar where you could step through the animation frame-by-frame, or play/rewind/stop/etc. This needs to be brought back. You may refer to earlier commits in this repository to check how that looked like.
| return NODE_ROLE_COLORS["lit"] | ||
| return NODE_ROLE_COLORS["unlit"] | ||
|
|
||
| def _animation_graph( |
There was a problem hiding this comment.
This entire function could do with more inline comments to explain the purpose of each block. Particular, lines 162-249 are not clear at a glance.
There was a problem hiding this comment.
This is my code. I wrote it a long time ago, even before lint and comments. There the geometry of the graph is calculated.
There was a problem hiding this comment.
I see, but I still think some comments should be added to explain the geometry. For example "this block aligns all unlit vertices in a row for Step IV", like that.
|
thanks @Newtech66 @gksmail , pushed fixes for everything yu mentioned above in the review, algebra name in the last frame title, a more distinguishable colour palette with a swatch legend, an interactive play/pause/step player instead of the gif, and comments through build_positions explaining the geometry. one note, i described the geometry blocks by what they do rather than the old "Step IV" labels since the current algorithm has no numbered steps, happy to reword if you prefer. |
Thanks! A small detail is that when a vertex is added to the end of a long line graph, the distance between the transferred lines increases. This is not serious, it would be good if the position of the graph and distances did not jump during animation. It will look smoother this way. I'm looking at algebra a17, with n=10 |
|
It will be necessary to rebase. After the adoption of PR on isomorphism, a conflict arose |
Gmin2
force-pushed
the
graph-trans-animation
branch
from
43bd8f6 to
f6f6181
Compare
|
@gksmail all frames now share one fixed viewport and spacing so the graph stays put during the animation, and also rebased |
Last time it was better. The image is no longer centered. It jumps even more. And the picture is not stretched. |
Gmin2
force-pushed
the
graph-trans-animation
branch
from
f6f6181 to
b97b1c2
Compare
|
my bad on the last push, i only checked that consecutive frames dont jump, not the overall composition, so the fixed viewport left the graph small and off-centre; now every frame is centred with one shared spacing and the labels are staggered so they stay readable at n=10: see the image |
|
there is lint error |
Gmin2
force-pushed
the
graph-trans-animation
branch
from
b97b1c2 to
37fe626
Compare
|
hey the lint err should be fixed now ! |
|
Thanks |
3 participants
Fixes #199
this brings back the animation feature for the classification algorithm, which got dropped when the algorithm was rewritten. it wraps the current Canonicalizer using the observer pattern: the canonicalizer is the publisher and emits an event at each step (lighting, contraction, attach, p/q merge, removals, replacements, dependent vertices), and a recorder subscribes to capture frames. nothing in the algorithm changes, the recorder only watches, so all existing tests still pass and a recorded run gives the same algebra as a plain one.
a renderer turns the recording into a gif with a color per node role, and the two worked examples in classification.rst (A-type 4*so(5) and B-type sp(4)) now have embedded animations plus a color legend. added tests for the frame invariants and a script to regenerate the doc gifs.