Remove unnecessary sqrt calculations

[RasmusLar]

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	N/A
Primary OS tested on	Ubuntu
Robotic platform tested on	Isaac Sim v4.2.0
Does this PR contain AI generated software?	Yes (GH Copilot)

---

Description of contribution in a few bullet points

• I replaced calculations which used hypot with a comparison with the square values, to avoid sqrt calculations to improve performance
• While writing unit tests, fixed discrepancy in goal checker orientation, which was checking for < tolerance instead of <= tolerance, as all the other limit checks are.
• Reduced tolerance time for the progress checker unit tests to 0.1 seconds, to reduce test runtime from ~17 to ~7 seconds.

Description of documentation updates required from your changes

• I think no documentation update is needed.

Description of how this change was tested

• I wrote unit tests and tested in Isaac sim for ~10 minutes

---

Future work that may be required in bullet points

• Other performance improvements that I can see, reduced copying values from const references

For Maintainers:

• Check that any new parameters added are updated in docs.nav2.org
• Check that any significant change is added to the migration guide
• Check that any new features OR changes to existing behaviors are reflected in the tuning guide
• Check that any new functions have Doxygen added
• Check that any new features have test coverage
• Check that any new plugins is added to the plugins page
• If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines	Coverage Δ
nav2_controller/plugins/pose_progress_checker.cpp	96.66% <100.00%> (ø)
nav2_controller/plugins/simple_goal_checker.cpp	100.00% <100.00%> (ø)
...av2_controller/plugins/simple_progress_checker.cpp	100.00% <100.00%> (+2.27%)	⬆️
nav2_controller/plugins/stopped_goal_checker.cpp	100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

[RasmusLar]

I just thought of an edge case, if radius_ was negative before, this would always return true.
This is no longer the case, should that be checked?

[RasmusLar]

See new commit as a suggestion to solve this, resolve if suggestion is good.

[RasmusLar]

Depending on how negative radius_ values should be checked, this should also be removed?

[RasmusLar]

See new commit as a suggestion to solve this, resolve if suggestion is good.

[SteveMacenski]

I replaced calculations which used hypot with a comparison with the square values, to avoid sqrt calculations to improve performance

I'm all for performance improvements, but can you highlight what the runtime metrics are for these improvements? Most of these operations are small and infrequent, so it kind of surprises me if this improves the performance in a measurable way. If there's not a measurable change, I'd prefer to not merge in additional complexity

Reduced tolerance time for the progress checker unit tests to 0.1 seconds, to reduce test runtime from ~17 to ~7 seconds.

I'm a huge fan of this though! Our Ci times are high and small improvements like this really add up quickly.

--

Thanks for the contribution! Sorry for the delay, I was on a trip to India and then I fell sick due to said trip 😆

[RasmusLar]

I'm all for performance improvements, but can you highlight what the runtime metrics are for these improvements? Most of these operations are small and infrequent, so it kind of surprises me if this improves the performance in a measurable way. If there's not a measurable change, I'd prefer to not merge in additional complexity
I only ran them separately, and locally it was like a 20% improvement on the function itself.

I guess the improvements are minor in the whole context of Nav2, but also I thought the complexity increase was minor. I guess the biggest issue would be readability.
I can try to set some time aside to do some tests to see how big of a difference it does in the context of the whole.

I'm a huge fan of this though! Our Ci times are high and small improvements like this really add up quickly.

I can separate the tests out into a different PR also when I get time

Thanks for the contribution! Sorry for the delay, I was on a trip to India and then I fell sick due to said trip 😆

Ouch, the one downside about traveling, hope you're better! 😄

[SteveMacenski]

I can separate the tests out into a different PR also when I get time

Please! That's easy for me to merge

I can try to set some time aside to do some tests to see how big of a difference it does in the context of the whole.

OK!