Skip to content

Add commutator functions#431

Merged
james-d-mitchell merged 7 commits into
libsemigroups:mainfrom
Joseph-Edwards:commutator
May 11, 2026
Merged

Add commutator functions#431
james-d-mitchell merged 7 commits into
libsemigroups:mainfrom
Joseph-Edwards:commutator

Conversation

@Joseph-Edwards
Copy link
Copy Markdown
Collaborator

This PR adds bindings and tests for the functions commutator and add_commutator_rule.

There was a slight hiccup here, because Pybind11 binds C++ chars as Python strings (with a few extra steps). This was causing some issues with overload resolution for the functions with the optional id parameter. Specifically, does add_commutator_rule(p, x, y, z) mean "add the commutator rule $[x, y] = \epsilon$ with inverses given by z" or does it mean "add the commutator rule $[x, y] = z$ with inverses inferred from the presentation"? It is reasonable to want to be able to do either of these things, but difficult to express your intent to Python, because id has the same type as the other parameters.

To get around this, I've made the parameter id a keyword-only argument (see PEP 3102 for more info). Therefore, if we want to specify a non-standard identity, we need to explicitly provide it as a keyword argument (e.g. add_commutator_rule(p, x, y, id=z)). I've tried to make this clear in the doc, but if I'm happy to add more detail if it's needed.

I don't think we've added any keyword-only arguments before, so I needed to make a few tweaks to some of our scripts and helpers to accommodate this.

@james-d-mitchell
Copy link
Copy Markdown
Member

Thanks @Joseph-Edwards, I'm happy with the keyword argument stuff you've proposed (in fact, in light of our recent conversation about this, I think maybe alphabet and inverses should be keyword args too. Obviously the CI failing needs to be fixed, but in principle I think this is good.

@james-d-mitchell james-d-mitchell merged commit 606bdad into libsemigroups:main May 11, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants