Calculate average reprojection error when loading SimCameraProperties #1673
Conversation
| double jsonErrorStdDevX = 0; | ||
| double jsonErrorStdDevY = 0; | ||
|
|
||
| if (jsonViewErrors != null && jsonErrorStdDev != null) { |
There was a problem hiding this comment.
Should I bother with a fallback for the existing logic?
There was a problem hiding this comment.
Probably unnecessary.
The load-by-path option here has not really been used since the SQLite storage was implemented and a calibration json was no longer easily accessible. Has this changed?
There was a problem hiding this comment.
There is still currently no longer any way to access a camera config file no. The calibration json file is still easy to get. Let's kill this feature if we don't need it anymore?
There was a problem hiding this comment.
Happy to yoink config file support if that's what we're looking to do here.
There was a problem hiding this comment.
I've deprecated the two applicable constructors to this feature.
There was a problem hiding this comment.
Given that we dropped support for this feature from the backend back in 2023, and the ctor is now a no-op, let's just delete them? Or open to your thoughts @amquake
| // error pixels in random direction | ||
| double error = avgErrorPx + rand.nextGaussian() * errorStdDevPx; | ||
| double errorAngle = rand.nextDouble() * 2 * Math.PI - Math.PI; | ||
| double errorX = avgErrorPxX + rand.nextGaussian() * errorStdDevPxX; | ||
| double errorY = avgErrorPxY + rand.nextGaussian() * errorStdDevPxY; | ||
| noisyPts[i] = | ||
| new Point(p.x + error * Math.cos(errorAngle), p.y + error * Math.sin(errorAngle)); | ||
| new Point(p.x + errorX, p.y + errorY); |
There was a problem hiding this comment.
IIRC the previous "average error" from calibration seemed to report the average absolute error. The previous way of estimating noise tries to use both the average (absolute) error and standard deviation which is pretty goofy. I'm not sure these numbers even really make sense to use in the first place, considering they measure solely the reprojection error of the calibrated instrinsics.
Anyway, it probably makes more sense to just consider the corner pixel normally distributed with given standard deviation. Meaning, just remove (deprecate) the average view error from SimCameraProperties. This will noticeably lower the noise currently seen by whatever values are used, but the simulation already seemed to overestimate it.
There was a problem hiding this comment.
I figured I'd try separating x and y noise because I didn't want to assume that the shape of the pixel noise distribution was circular, but with removing support for file based configs there isn't much of a reason to keep it around. I found it to be useful because we could leverage existing data rather than come up with those values independent of calibration data.
In this changeset I opted to revert the separation of the two because otherwise we'd be deprecating setCalibError(double avgErrorPx, double errorStdDevPx) and introducing setCalibError(double errorStdDevPxX, double errorStdDevPxY). It's always easy to reintroduce at a better time,
Currently, exported photonvision camera configs cannot be loaded for sim use. Part of this is because SimCameraProperties expects the provided config file to include both the average and standard deviation of reprojection error measurements. Instead, the config file contains individual reprojection error measurements.
This PR improves sim support for the current exported camera calibration file from PhotonVision, additionally separating the X and Y pixel noise.