Skip to content

fix(trajectory): normalize quintic profile time - #736

Open
Afloat16 wants to merge 4 commits into
petercorke:mainfrom
Afloat16:fix-quintic-time-conditioning-20261011
Open

Afloat16 wants to merge 4 commits into
petercorke:mainfrom
Afloat16:fix-quintic-time-conditioning-20261011

Conversation

@Afloat16

Copy link
Copy Markdown
Contributor

Summary

quintic(1, 2, 1000) starts near zero instead of 1, while a one-millisecond profile remains near 1.5 at both endpoints. The unscaled least-squares system loses numerical accuracy or rank and discards boundary constraints.

Evaluate the quintic in normalized time using a factored Hermite basis. Preserve endpoint velocity units, zero endpoint acceleration, supplied timestamps, sample shapes, the zero-duration result and existing coefficient-type errors. Promote low-precision samples before normalization and basis evaluation, retaining supported complex and extended sample types. Add independent polynomial/derivative, 70-digit float32, signed-velocity and native multi-axis controls. Use NumPy testing calls for the Boolean checks so they remain active under Python optimization.

Validation

  • Exact base: 968ed048639c01c9032e256565731e2703de77fe. Updated tests on that base: 27 failed, 35 passed. Fixed trajectory tests: 76 passed, 14 subtests.
  • Linux/Python 3.12.14: rerun supported native suite passes with 906 passed, 3 existing skips, 17 subtests. Rerun core without Swift: 897 passed, 12 expected skips, 17 subtests. Matching unchanged native-base extensions were reused and verified active.
  • The 13 Boolean expressions are unchanged by AST comparison. All 62 parametrized test cases run directly under normal Python and -O: 172 actual Boolean checks pass and 172 forced-false calls raise AssertionError in each mode, without pytest assertion rewriting.
  • Production code is unchanged from the previously validated revision. Its complete coverage run passes with 906 passed, 3 existing skips and 17 subtests, covering every executable quintic line. Coverage initially missed the existing C-versus-Python timing threshold; the unchanged complete serial repeat passes. That failure and the passing baseline timing control are retained. Coverage was not rerun for this test-only revision.
  • Ruff formatting, updated-test lint and git diff --check pass. Rerun whole-project lint adds no diagnostics; existing diagnostics remain at 1211 versus 1214 on the base.
  • The previous configured Sphinx HTML builds are reused for the unchanged production source. They succeed while repeating the baseline's 20 existing runblock errors at three ETS/ETS2 locations; those examples are not claimed as passing. Documentation was not rebuilt for this test-only revision.
  • Numerical regressions exercise selected durations from 0.001 to 1,000,000. Other operating systems, Python versions and browser/Pyodide execution were not tested locally.

Related issue

None.

Checklist

  • PR title follows Conventional Commits
  • Tests pass locally, with existing skips listed above
  • Added tests for this change
  • Changed function and new tests use modern type hints
  • Docstring updated in reST style
  • Change is focused
  • No project-specific data or notebooks added

@codacy-production

codacy-production Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 65 complexity · 9 duplication

Metric Results
Complexity 65
Duplication 9

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (f28437f) to head (fd83877).

Files with missing lines Patch % Lines
src/roboticstoolbox/tools/trajectory.py 0.00% 28 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #736   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        143     143           
  Lines      14667   14693   +26     
=====================================
- Misses     14667   14693   +26     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Afloat16
Afloat16 marked this pull request as ready for review October 13, 2026 08:14
@petercorke

Copy link
Copy Markdown
Owner

Thanks for your interest in RTB, and for taking the time to create a PR. Once again I'm afraid your summary is just too cryptic. You declare that you are using it AI which is fine, just prompt it: to write summaries for somebody who is solo maintaining multiple large open source projects and likes clear/simple examples of the issue and a gentle onramp for the solution.

I see the problem but I'm reluctant to discard this function which I think has pedagogical value, as does its mode of failure. I'd propose this alternative:

  • in existing quintic_func test the condition of X and raise an error for this case, and describe the issue clearly in the docstring
  • create a parallel function quintic_hermite_func or whatever that implements your fix.

Restore the six-equation teaching solver with a numerical-rank guard.
Export the normalized-time Hermite function and use it for sampled quintic trajectories.
Cover caller routing, endpoint derivatives and representable duration scaling.
@petercorke

Copy link
Copy Markdown
Owner

Thanks for following up, it maybe that the hermit code becomes mainstream quintic_func one day and the old LS approach is kept commented out for pedagogy -- that algorithm is used in my book which is why I am keen to retain it. I didn't realise the conditioning went to hell quite so quickly. Thanks again.

petercorke added a commit that referenced this pull request Oct 14, 2026
…es (#768)

Annotates the tools, demo, bin, models, ets and robot packages and holds them to disallow_untyped_defs. BaseETS is generic over its element type (BaseETS[ET], BaseETS[ET2]), replacing the self-type overloads. 68 unused pyright ignores removed, module/class name clashes made explicit. Left on the mypy to-do list: tools/trajectory.py (open #736), mobile/ParticleFilter.py (open #735), the two unused DHFactor modules and a stale test in tools/urdf/tests. Behaviour unchanged except on paths that already raised.
Retain the teaching solver and Hermite API while preserving upstream
mstraj derivatives and import cleanup.

This branch has not been deployed

No deployments
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