Add support for the S1 (circle) manifold and its tangent space

[edxmorgan]

Support for the 1-D circle manifold (S1) by:

• Implementing S1 class with wrapping, multiplication, inverse, log/exp maps
• Adding S1Tangent for tangent-space operations
• Exporting S1 and S1Tangent in liecasadi/init.py
• Extending generic operations to include S1 and S1Tangent
• Adding unit tests for identity, multiplication, subtraction, log/exp consistency

[Giulero]

Thanks @edxmorgan for the PR! This is indeed a nice addition.
Sorry for the late response, I missed the notification.
Rewieving now!

[Copilot]

Pull Request Overview

This PR adds support for the 1-D circle manifold (S1) and its tangent space, integrates it into the package API, and provides corresponding tests.

• Introduces S1 and S1Tangent classes with wrapping, multiplication, inverse, and exp/log maps
• Updates exports and generic operations to include S1 and S1Tangent
• Adds unit tests for identity, multiplication, subtraction, and log/exp consistency

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tests/test_liecasadi.py	Adds tests for S1 and S1Tangent functionality
src/liecasadi/s1.py	Implements S1 manifold and S1Tangent operations
src/liecasadi/init.py	Exports S1 and S1Tangent
README.md	Updates documentation to include S1 support

Comments suppressed due to low confidence (2)

src/liecasadi/s1.py:24

• Using a raw float for the identity angle may cause type mismatches with CasADi symbolic types; consider using cs.DM(0) or a CasADi scalar constructor to maintain consistency.

        return S1(0.0)

src/liecasadi/s1.py:78

• [nitpick] For consistency with S1.as_angle(), consider renaming value() to as_value() or as_scalar() to clearly convey its purpose.

    def value(self) -> Scalar:

[Copilot]

[nitpick] Remove or utilize the unused variables S1_angle, myS1, S1_tangent_ang, and myS1Tang in the test file to eliminate noise and improve maintainability.

Suggested change

	# S1 objects
	S1_angle = (np.random.rand() - 0.5) * 2 * np.pi
	myS1 = S1(S1_angle)
	# S1Tangent objects
	S1_tangent_ang = (np.random.rand() - 0.5) * 2 * np.pi
	myS1Tang = S1Tangent(S1_tangent_ang)
	# Removed unused variables S1_angle, myS1, S1_tangent_ang, and myS1Tang.

[edxmorgan]

S1_angle, myS1, S1_tangent_ang, and myS1Tang unused variables removed

[edxmorgan]

@Giulero review required. thanks

[Giulero]

Hi @edxmorgan ! Left a comment related to the function that wraps to pi - see the review. What do you think?

[edxmorgan]

Hi @edxmorgan ! Left a comment related to the function that wraps to pi - see the review. What do you think?

@Giulero I'm sorry but i am unable to find this comment. can you please point it out with a quote reply

[Giulero]

No worries! I mean this comment #19 (comment)

[edxmorgan]

okay . I see. I am able to partially read it like this. I think github has a bug (https://github.com/orgs/community/discussions/30638). The comment itself does not exist for me.
.

[Giulero]

Uh I see! This is what i wrote, related to https://github.com/edxmorgan/liecasadi/blob/e02f79e6d090ced2e95bf72ae0d41540eee0e6c9/src/liecasadi/s1.py#L7-L9

---

I'm not sure this function is continuous and differentiable :/
A possible solution could be to use something like

def _wrap_to_pi(angle: Scalar) -> Scalar:
	s = cs.sin(angle)
	c = cs.cos(angle) 
	return cs.atan2(s, c)

which should be continuous.
The issue is when angle == 0.0, but maybe adding a small tolerance np.finfo().eps could solve this.
@edxmorgan what do you think?

[edxmorgan]

Hi @Giulero, excellent catch!

atan2(cs.sin(θ), cs.cos(θ)) stays continuous on (−π, π] because sin²θ + cos²θ = 1, so the pair never reaches (0, 0). At θ = 0 the call is atan2(0, 1), which returns 0 cleanly, so an ε-guard isn’t necessary. The only nondifferentiable points are the inherent branch cuts at ±π.

I’ll replace the current implementation with this function. Thanks for spotting it! 🚀

[Giulero]

Looks good to me! Thanks @edxmorgan :)
I think we can merge

[Giulero]

I'm not sure this function is continuous and differentiable :/
A possible solution could be to use something like

def _wrap_to_pi(angle: Scalar) -> Scalar:
	s = cs.sin(angle)
	c = cs.cos(angle) 
	return cs.atan2(s, c)

which should be continuous.
The issue is when angle == 0.0, but maybe adding a small tolerance np.finfo().eps could solve this.
@edxmorgan what do you think?

cc @edxmorgan

[Giulero]

End of line missing!

[Giulero]

With a new module, it could make sense to split the tests into multiple files. But we can do it in a different pr

[edxmorgan]

right! Splitting the tests into separate files will make it look organized. I’ll open a PR for that.