Skip to content

Add shirokov_inverse and hitzer_inverse#530

Merged
utensil merged 4 commits intopygae:masterfrom
spinjo:more_inverse
Dec 26, 2024
Merged

Add shirokov_inverse and hitzer_inverse#530
utensil merged 4 commits intopygae:masterfrom
spinjo:more_inverse

Conversation

@spinjo
Copy link
Contributor

@spinjo spinjo commented Dec 24, 2024

Add shirokov_inverse and hitzer_inverse functions, based on the clifford package implementation of the shirokov_inverse and hitzer_inverse.

closes #529

@spinjo
Copy link
Contributor Author

spinjo commented Dec 24, 2024

I did a few tests on dim=3 and dim=4, but not on dim>=5 because the code was so slow. Happy about ideas on how to speed it up! Plus I think it would make sense to add unit tests for the inverse.

@utensil
Copy link
Member

utensil commented Dec 25, 2024

For a first PR, dim 3 to 4 tests suffice. Slowness of GAlgebra above 4 dims are observed in other cases too.

There are a few issues to explore the possibilities to speed up GAlgebra, but none is sufficiently mature.

I'll take a look at the code ASAP. Thanks for the PR! ❤️

Uk = self
for k in range(1, N):
Ck = (N / k) * Uk.scalar()
adjU = Ck - Uk
Copy link
Member

Choose a reason for hiding this comment

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

I can't immediately see why you negated it here and negated it back below.

Copy link
Contributor Author

@spinjo spinjo Dec 25, 2024

Choose a reason for hiding this comment

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

Thats not in line with the clifford code, but in line with the Shirokov paper. The variables adjU and Uk sometimes differ by a sign in the clifford code for some reason.

Copy link
Member

Choose a reason for hiding this comment

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

👌

@utensil
Copy link
Member

utensil commented Dec 25, 2024

Looks correct and good to me (cross-checked with clifford and the papers), except for one comment.

Can you add your tests for dim 3-4 as tests in CI? It's fine to add them as notebooks or vanilla tests.

@utensil utensil added enhancement component: core Ga, Mv, Metric, etc state: needs changelog Needs a changelog entry before the next release. Remove this label when the changelog is done. labels Dec 26, 2024
@utensil utensil merged commit b071b8d into pygae:master Dec 26, 2024
1 check passed
@EelcoHoogendoorn
Copy link

I did a few tests on dim=3 and dim=4, but not on dim>=5 because the code was so slow. Happy about ideas on how to speed it up! Plus I think it would make sense to add unit tests for the inverse.

Doing the hitzer ops recursively, rather than greedily forming the product of all involutions, can be much faster. When working recursively the number of components tends to drop dramatically at each step.

@koraxkorakos koraxkorakos mentioned this pull request Dec 27, 2024
@utensil utensil mentioned this pull request Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: core Ga, Mv, Metric, etc enhancement state: needs changelog Needs a changelog entry before the next release. Remove this label when the changelog is done.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shirokov inverse?

3 participants