You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In December 2023, @demul opened PR #1695 to add a Cal3DS2_k3 camera calibration model. The contribution included the calibration model, analytic Jacobians, tests, serialization support, and Python wrapper integration.
I want to explicitly acknowledge and thank @demul for this thoughtful and substantial contribution, as well as for patiently following up on the design and CI questions.
I also owe @demul an apology. I was working at a startup while this was happening and was not able to give the pull request the timely review and support it deserved. I am sorry that it was left waiting without a clear resolution.
Why move this to a discussion
The original branch is now several years old, conflicts with current develop, and predates substantial API and wrapper changes. I am closing the original pull request as stale, not because the capability lacks value or because the contribution was unappreciated.
The main design question raised in the review is still worth resolving: should GTSAM add a dedicated 10-parameter Cal3DS2_k3 type, or should we generalize the existing distortion model to support a configurable number of radial coefficients while preserving compatibility with Cal3DS2?
Reviving the work
I am willing to support reviving this feature from current develop, including helping settle the API design and reviewing a replacement pull request. The implementation and tests in PR #1695 should be valuable reference material, and I would be especially happy to work with @demul again if they are interested in returning to it.
A revival should agree on the public API first, then cover:
backward compatibility with Cal3DS2;
fixed-size Jacobians and manifold dimensions;
C++ and Python wrapper exposure;
serialization compatibility; and
focused unit and numerical-derivative tests.
If there is interest in taking this forward, please reply here so we can agree on the design before code is rewritten.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
Background
In December 2023, @demul opened PR #1695 to add a Cal3DS2_k3 camera calibration model. The contribution included the calibration model, analytic Jacobians, tests, serialization support, and Python wrapper integration.
I want to explicitly acknowledge and thank @demul for this thoughtful and substantial contribution, as well as for patiently following up on the design and CI questions.
I also owe @demul an apology. I was working at a startup while this was happening and was not able to give the pull request the timely review and support it deserved. I am sorry that it was left waiting without a clear resolution.
Why move this to a discussion
The original branch is now several years old, conflicts with current develop, and predates substantial API and wrapper changes. I am closing the original pull request as stale, not because the capability lacks value or because the contribution was unappreciated.
The main design question raised in the review is still worth resolving: should GTSAM add a dedicated 10-parameter Cal3DS2_k3 type, or should we generalize the existing distortion model to support a configurable number of radial coefficients while preserving compatibility with Cal3DS2?
Reviving the work
I am willing to support reviving this feature from current develop, including helping settle the API design and reviewing a replacement pull request. The implementation and tests in PR #1695 should be valuable reference material, and I would be especially happy to work with @demul again if they are interested in returning to it.
A revival should agree on the public API first, then cover:
If there is interest in taking this forward, please reply here so we can agree on the design before code is rewritten.
All reactions