Skip to content

first pass at adding ocircle operator to SE(3) with corresponding tests#28

Merged
angadbajwa merged 3 commits intomainfrom
feature/ocircle
Apr 30, 2026
Merged

first pass at adding ocircle operator to SE(3) with corresponding tests#28
angadbajwa merged 3 commits intomainfrom
feature/ocircle

Conversation

@angadbajwa
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds the SE(3) “ocircle” operator to the NumPy implementation and introduces targeted tests to validate the odot/ocircle identities against SE3.wedge.

Changes:

  • Add SE3.ocircle to pymlg/numpy/se3.py.
  • Add tests/numpy/test_se3.py with identity checks for odot and ocircle.

Reviewed changes

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

File Description
pymlg/numpy/se3.py Adds NumPy SE3.ocircle operator implementation.
tests/numpy/test_se3.py Adds group-specific tests for odot/ocircle identities.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/numpy/test_se3.py
Perform any other group-specific tests that are not part of the standard tests.
"""

from pymlg import SE3 as G
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

This test module lives under tests/numpy/, but it imports SE3 from the top-level pymlg package (the C++ wrapper by default). To ensure this exercises the pure NumPy implementation and matches the pattern used by tests/numpy/test_standard_numpy.py, import SE3 from pymlg.numpy instead (and keep backend-specific tests in their respective folders).

Suggested change
from pymlg import SE3 as G
from pymlg.numpy import SE3 as G

Copilot uses AI. Check for mistakes.
Comment thread tests/numpy/test_se3.py Outdated
Comment on lines +23 to +24
# test_euler()
# test_quaternion()
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

The __main__ block includes commented-out calls to test_euler()/test_quaternion(), but those functions are not defined in this file. Please remove these stale comments (or add the corresponding tests here) to avoid confusion when running the module directly.

Suggested change
# test_euler()
# test_quaternion()

Copilot uses AI. Check for mistakes.
Comment thread pymlg/numpy/se3.py
Comment on lines +168 to +175

@staticmethod
def ocircle(b):
b = np.array(b).ravel()
X = np.zeros((6, 4))
X[3:6, -1] = b[0:3]
X[0:3, :3] = SO3.odot(b[0:3])
return X
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

ocircle is added on the NumPy SE3, but the project README states the user API should be the same across implementations (notably NumPy vs JAX). Currently pymlg/jax/se3.py (and torch) has odot but no ocircle, so this introduces an API mismatch. Either add ocircle to the other backends (at least JAX, per README) or update the API parity promise/documentation.

Suggested change
@staticmethod
def ocircle(b):
b = np.array(b).ravel()
X = np.zeros((6, 4))
X[3:6, -1] = b[0:3]
X[0:3, :3] = SO3.odot(b[0:3])
return X

Copilot uses AI. Check for mistakes.
@angadbajwa angadbajwa merged commit 4e3c19b into main Apr 30, 2026
6 checks passed
@angadbajwa angadbajwa deleted the feature/ocircle branch April 30, 2026 19:12
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