Brooker/port srif localisation - #50
Open
sebastianbrooker2 wants to merge 8 commits into
Open
sebastianbrooker2 wants to merge 8 commits into
sebastianbrooker2 wants to merge 8 commits into
Conversation
Direct port of the new files from upstream NUbots PR NUbots#1823 (branch tumminello/localisation, commit a83236b): the gaussian_filtering math library, the FieldLocalisationSRIF module, the OutOfFieldFeatures message, and the new NUsight2 visualisation components. Unmodified from upstream aside from the file selection. Co-authored-by: Vincent Tumminello <vincent.tumminello@uon.edu.au>
…iverged files The remaining upstream changes (autodiff dependency in the Dockerfile, the FieldIntersection.pixel_centre field, the NaN-safety clamps and project_pixel/unproject_pixel helpers in projection.hpp, and populating pixel_centre in Yolo.cpp) couldn't be applied as a straight patch since these files have diverged from upstream. Applied by hand, matching the PR's hunks exactly. Skipped the PR's from-source Boost 1.87 install -- K1's Dockerfile already installs libboost-dev via apt and nothing ported here needs a newer version. Co-authored-by: Vincent Tumminello <vincent.tumminello@uon.edu.au>
Adds the ported module (see previous two commits, from NUbots#1823) alongside the existing FieldLocalisationNLopt in robocup.role, test/localisation.role and webots/localisation.role, so it runs and can be compared against the existing estimator rather than replacing it.
The previous commit brought in uncertainty_ellipse.tsx and out_of_field.ts as new files, but missed the PR's smaller edits to existing NUsight files that actually reference them: visibility toggles and model fields for the uncertainty ellipse (controller/model/robot_model/view under localisation/), and the network handler plus camera model/view wiring for the out-of-field overlay (vision/). Applied by hand against K1's diverged copies of these files, same as the previous commit. Co-authored-by: Vincent Tumminello <vincent.tumminello@uon.edu.au>
Both it and the newly-ported FieldLocalisationSRIF emit message::localisation::Field, and every consumer of that message (WalkToBall, GameController, Overview, purpose modules, NUsight) just takes whichever was emitted most recently via With<Field>. Running both at once means those consumers nondeterministically flip between the two estimators. Disabling NLopt here so SRIF can be evaluated on its own; re-enable it (and comment out SRIF instead) to go back to the old behaviour.
Every other install-from-source RUN in this Dockerfile passes this for build parallelism; the autodiff line (ported from upstream PR NUbots#1823, which doesn't have this K1-specific convention) was missing it. Unrelated to the "autodiff/forward/dual.hpp: No such file" build failure reported -- that's the container needing a rebuild to pick up this dependency at all, not a flag issue.
Went through the still-open (non-outdated) review threads on upstream NUbots#1823 and applied the ones that are concrete, safe, and don't require re-architecting anything: - Tom0Brien: README.md used Rfb where every other symbol in the same table uses T for torso; renamed to Rft. - jpptm: removed a dead 2-line comment in MeasurementBodyRates.cpp (matches the reviewer's literal blank-line suggestion). - jpptm: tangentBasis -> tangent_basis, and logLikelihoodImpl -> log_likelihood_impl across every measurement class (not just the one flagged instance, to keep the sibling classes consistent). - jpptm ("make it snake case", adjacent to the gate/confidence comments): renamed the whole MeasurementFieldLandmarks::Options field set to snake_case (sigma_angular, gate_angle, gate_yaw_scale, gate_angle_max, min_confidence, inlier_probability, confidence_reference) and updated the one place that assigns them from config in FieldLocalisationSRIF.cpp. Config YAML keys were already snake_case, so this doesn't touch config. - jpptm ("a bunch of commented code in there" in funcmin.hpp): removed 5 duplicated dead alternate-threshold comments, one larger dead alternate-implementation block (QR-based Newton step alternative, never used), and a stray commented-out typedef. - Tom0Brien/jpptm ("rename the slam directory"): the directory was renamed to gaussian_filtering at some point but the header guards were never updated to match; renamed UTILITY_SLAM_* -> UTILITY_GAUSSIAN_FILTERING_* across all 10 headers in the utility. Left alone (subjective, unresolved-by-design, or too large/risky to attempt without breaking things): the "do we need this whole file" architecture questions on Pose.hpp/funcmin.hpp/GaussianBase.hpp (Vincent already answered these in the thread -- Pose<Scalar> is needed for autodiff dual-type instantiation); the hand-tuned erfcx polynomial in GaussianBase.hpp jpptm called "an abomination" (no concrete alternative was suggested, and it's a real minimax numerical approximation, not arbitrary mess); the config file's comment verbosity (subjective, and the content is genuinely load-bearing context, not restated code); the "abstract this across measurement classes" suggestions (real refactors, not safe to do speculatively); and the from-source Boost Dockerfile question, which doesn't apply here since we never added it.
…ometry output or htw difference. Remove odometry twist buffer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Things the PR does go here