Skip to content

Conversation

@bgrigsby8
Copy link

No description provided.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Nov 10, 2025
@dgottlieb dgottlieb self-requested a review November 11, 2025 16:10
@dgottlieb
Copy link
Member

@bgrigsby8 the test you added seems to be failing. How did you determine the correct answer?

}

func TestOrientationVectorPoleRadius(t *testing.T) {
ov := &OrientationVector{Theta: 90.2029644505, OX: 0.0050164674, OY: 0.0079070413, OZ: 0.9999561559}
Copy link
Member

Choose a reason for hiding this comment

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

I think you meant to use OrientationVectorDegrees here. The test still fails, but much more reasonably:

=== RUN   TestOrientationVectorPoleRadius
    orientation_test.go:161: Expected '90.20295555178383' to almost equal '90.2029644505' (but it didn't)!

I am confused though why if I instead change everything to radians, the test failure gets worse. Code:

	ov := &OrientationVector{Theta: 1.5743387247206226, OX: 0.0050164674, OY: 0.0079070413, OZ: 0.9999561559}
	q := Quaternion(ov.Quaternion())
	composedOv := q.OrientationVectorRadians()
	test.That(t, composedOv.Theta, test.ShouldAlmostEqual, ov.Theta, 0.000001)

Gives:

=== RUN   TestOrientationVectorPoleRadius
    orientation_test.go:162: Expected '0.009364093511798814' to almost equal '0.00501646740007625' (but it didn't)!

Copy link
Member

Choose a reason for hiding this comment

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

(I assume theta is the only thing in degrees/radians, but I don't really know any of this stuff)

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this test was meant to fail. I was meant to use OrientationVectorDegrees, but this orientation_test.go:162: Expected '0.009364093511798814' to almost equal '0.00501646740007625' (but it didn't)! is what im experiencing and real life and wanted to create a test to capture the failure. I'm working with @nicksanford on this

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Nov 11, 2025
@github-actions
Copy link
Contributor

Availability

Scene # viamrobotics:main viamrobotics:adjust-ov-pole-radius Percent Improvement Health
1 100% 100% 0%
2 100% 100% 0%
3 100% 100% 0%
4 100% 100% 0%
5 100% 100% 0%
6 100% 100% 0%
7 100% 100% 0%
8 100% 100% 0%
9 100% 100% 0%
10 100% 100% 0%

Quality

Scene # viamrobotics:main viamrobotics:adjust-ov-pole-radius Percent Improvement Probability of Improvement Health
1 1.31±0.00 1.31±0.00 -0% 45%
2 0.90±0.00 0.90±0.00 -0% 50%
3 5.85±1.25 5.85±1.25 -0% 50%
4 3.23±0.41 3.23±0.41 -0% 50%
5 9.54±2.80 9.99±3.75 -5% 46%
6 9.92±3.04 9.87±3.06 1% 50%
7 5.85±2.79 5.88±2.77 -0% 50%
8 0.90±0.00 0.90±0.00 -0% 50%
9 4.38±0.23 4.29±0.21 2% 62%
10 12.84±0.41 12.84±0.41 -0% 50%

Performance

Scene # viamrobotics:main viamrobotics:adjust-ov-pole-radius Percent Improvement Probability of Improvement Health
1 0.02±0.00 0.02±0.00 1% 53%
2 0.06±0.01 0.06±0.01 -2% 45%
3 0.08±0.06 0.08±0.06 -8% 47%
4 1.29±0.09 1.27±0.06 1% 55%
5 1.94±0.35 2.40±1.51 -23% 38%
6 2.28±0.64 2.25±0.67 1% 51%
7 2.47±0.74 2.43±0.71 1% 51%
8 0.06±0.00 0.06±0.00 1% 55%
9 2.17±0.15 2.21±0.16 -2% 43%
10 6.30±1.04 6.33±1.05 -0% 49%

The above data was generated by running scenes defined in the motion-testing repository
The SHA1 for viamrobotics:main is: 5385bdb0a6d8e617a53c351316b62a26a2b93ad4
The SHA1 for viamrobotics:adjust-ov-pole-radius is: 5385bdb0a6d8e617a53c351316b62a26a2b93ad4

  • 10 samples were taken for each scene

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test This pull request is marked safe to test from a trusted zone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants