Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 16, 2025

  • Review mathematical correctness of sign handling in Expmap
  • Remove incorrect sign-flipping logic that didn't fix determinants
  • Implement mathematically sound error handling for negative determinants
  • Add proper test assertions to verify det(T) = 1
  • Add tests for large tangent vectors with exception handling
  • Validate all changes with tests
  • Fix Mac CI boost installation issue by adding brew update

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Remove incorrect sign-flipping logic that didn't actually fix negative determinants
- Replace with mathematically sound approach: throw error on non-positive determinants
- Remove absurdly large tolerance (1e3) that would accept det ranging from e^-1000 to e^1000
- Add proper test assertions to verify det(T) = 1 for normal cases
- Add separate tests for large tangent vectors with proper exception handling

Co-authored-by: dellaert <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 16, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Fix SL4 to determinant issue Fix mathematically incorrect determinant sign handling in SL4::Expmap Dec 16, 2025
Copilot AI requested a review from dellaert December 16, 2025 15:40
@dellaert
Copy link
Member

Mac CI with boost does not pass. Please add a “brew update” step before upgrading or installing boost. One line change please.

Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

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

Mac CI with boost does not pass. Please add a “brew update” step before upgrading or installing boost. One line change please.

Add explicit brew update step before installing boost to prevent CI failures on macOS.

Co-authored-by: dellaert <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 16, 2025

Mac CI with boost does not pass. Please add a “brew update” step before upgrading or installing boost. One line change please.

Added brew update before boost installation in commit 57e7345.

Mac CI with boost does not pass. Please add a “brew update” step before upgrading or installing boost. One line change please.

Added brew update before boost installation in commit 57e7345.

Copilot AI requested a review from dellaert December 16, 2025 16:03
@dellaert dellaert marked this pull request as ready for review December 17, 2025 04:07
@dellaert dellaert merged commit 84e1116 into fix/sl4 Dec 17, 2025
34 of 64 checks passed
@dellaert dellaert deleted the copilot/sub-pr-2317 branch December 17, 2025 04:08
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.

2 participants