Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multigraph rules fixes: bialgebra #248

Merged
merged 18 commits into from
Jul 9, 2024

Conversation

RazinShaikh
Copy link
Contributor

No description provided.

@RazinShaikh RazinShaikh marked this pull request as ready for review July 9, 2024 00:34
@RazinShaikh
Copy link
Contributor Author

I have covered the three implementations of bialgebra in rules.py, basicrules.py and editor_actions.py. Let me know if that looks alright to you.

@jvdwetering
Copy link
Collaborator

I don't claim to fully understand what is going on here, but if you say that this indeed correctly implements the bialgebra rule and deals with all the weird edge cases you can have when you work with a multi-graph, then I'm happy to merge it.

@RazinShaikh
Copy link
Contributor Author

I have tested with the weird cases and I believe it is correct. I can't guarantee that I haven't missed anything but previously the bialgebra rule was completely broken so this is at least an improvement.

Few comments:

  1. The bialgebra in editor_actions.py is the one previously in basicrules.py modified to work with multiple edges and self-loops. Whenever there are no multi-edges, its behavior should match the one previously.
  2. I have made the basicrules.py version use the bialgebra from editor_actions.py so there is less duplication.
  3. The rules.py version of bialgebra is supposed to be used with automated rewrite strategies and it expects the number of vertices to always decrease. Whenever we have multiple edges between two nodes or self-loops, the number of vertices either stays the same or increases. So I have simply added a condition in the matcher that doesn't match any time there are multiple edges between those nodes or self-loops.

@jvdwetering jvdwetering merged commit 716c3e9 into zxcalc:master Jul 9, 2024
3 checks passed
@RazinShaikh RazinShaikh deleted the multigraph-rules-fixes branch July 9, 2024 18:33
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.

3 participants