Pinhole model support in C API for mrcal_rectified_system

[akonneker]

See here

I just stumbled over this while embedding mrcal rectification map calculation into some C++ code. I had previously used rectified_system with a pinhole model in the python api.

Do you have a timeline for when you might add this support?
Or perhaps you have some advice about an easier workaround than just using the python API?

[dkogan]

Hello. This is very low priority for me since pinhole rectification is strictly worse than latlon rectification in every way: https://mrcal.secretsauce.net/stereo.html

If you need this, you should implement it! It wouldn't be that difficult, and I would appreciate the contribution. Port the relevant logic in mrcal._rectified_system_python() to the C mrcal_rectified_system() function. There already exists a test that makes sure that the not-yet-written port is correct: test-rectification-maps.py.

Thanks!

[akonneker]

I'm happy to add the functionality, but can you help me understand how that test works to call the c function?
As-is it happily passes, when I would expect it to fail if it actually called the c version of the function.

[akonneker]

Ah, I suppose I need to undo this behavior in stereo.py:

mrcal/mrcal/stereo.py

Line 511 in 71c89c4

NOTE: THE C IMPLEMENTATION HANDLES LENSMODEL_LATLON only. The

[dkogan]

Yep. The python function mrcal.rectified_system() is the outer logic, and it makes sure to call the Python code when using pinhole models:

mrcal/mrcal/stereo.py

Line 422 in 71c89c4

if rectification_model == 'LENSMODEL_PINHOLE':

So you're remove that if statement entirely. Right after that, it calls mrcal._mrcal._rectified_system(), which routes to the C code.

[akonneker]

So I have it implemented, and test-stereo.py and test-rectification-maps.py are now passing.

The primary issue is how to replace the root-finding provided by numpy with something in c. I found this as a possible solution: https://github.com/yairchu/quartic. It's BSD licensed and only two files (for the c implementation), so it seems to me to be a reasonable solution to include the files directly in mrcal, but that's up to you to decide.

I found another issue when working on this: there are missing docstring files that are referenced in mrcal-pywrap.c that don't exist in the repo:
measurement_index_points_triangulated
num_measurements_points_triangulated
decode_observation_indices_points_triangulated

I just commented those out to move forward, but I thought you should know.

[dkogan]

akonneker ***@***.***> writes:

So I have it implemented, and test-stereo.py and test-rectification-maps.py are now passing.

Cool!

The primary issue is how to replace the root-finding provided by numpy with something in c. I found this as a possible solution: . It's BSD licensed and only two files (for the c implementation), so it seems to me to be a reasonable solution to include the files directly in mrcal, but that's up to you to decide.

Solving a quartic should be a standard-enough thing that we shouldn't need to pull in any weird libraries. My suggestion would be to - Look at the code to understand what it does and why it does it. Maybe we don't NEED to solve the quartic? It has been a while, so I don't remember all the details, but I think this is intended to compute the rectified image dimensions from pixels_per_deg. For pinhole rectification this is poorly-defined: the pixel resolution changes dramatically across the image. So maybe we can formulate this in a slightly different way that doesn't need a quartic solve? - How do other toolkits do this? Do they simply let the user give them the rectified image size, and call it done? - If we really must solve the quartic, and there's no way to simplify the problem, let's use libdogleg. This is what mrcal is already using to solve the calibration problem and a few other auxillary things. It's intended to solve big, sparse least-squares problems, but we can use it here to solve a dense least-squares problem with one parameter (the argument of the quartic) and one measurement (the value of the quartic at that argument). I think that should work. Sample usages: https://github.com/dkogan/libdogleg/blob/c971ea43088d286a3683c1039b9a85f761f7df15/sample.c#L302C48-L303C41 https://github.com/dkogan/mrcal/blob/71c89c4e9f268a0f4fb950325e7d551986a281ec/mrcal.c#L3238

I found another issue when working on this: there are missing docstring files that are referenced in mrcal-pywrap.c that don't exist in the repo: measurement_index_points_triangulated num_measurements_points_triangulated decode_observation_indices_points_triangulated

Yeah. This is one of the many things that will eventually end up in mrcal 3.0. It's nowhere near done, and the missing docstrings are a part of that.

I just commented those out to move forward, but I thought you should know.

That's ok for now. Once you have a workable patch we'll need to figure out what to do. That could be you using the current mrcal git (which would include your code), or maybe I should release mrcal 2.5 at some point that will include the stuff that's done.

[dkogan]

Hi. Are you still trying to do this? Do you need help?

[akonneker]

Sorry for the delayed response. My quick hack satisfied my initial need and I've been pretty busy at work since then. I'll come back to this in another week or so when things slow down.