Add Cal3DS2_k3 distortion model

[demul]

This PR add new Calibration class Cal3DS2_k3.
It's almost identical to the Cal3DS2 class, except that its radial distortion coefficients are not only k1, k2 but also k3.

This PR also add test cases for Cal3DS2_k3. (testCal3DS2_k3.cpp)
I've performed unit test on the docker image(https://github.com/borglab/gtsam/tree/develop/docker/ubuntu-gtsam) and passed.

It has been confirmed that it is well compatible and converge well with factors like "gtsam_unstable/slam/ProjectionFactorPPPC.h" on my custom example script.

Please let me know if this feature already existed, or if there is any suggestion for merge.

[demul]

Hello @dellaert @varunagrawal
It seems that my PR didn't pass Python CI action.
Is there any way to do CI test on my own local PC?

and, If possible, could you please let me know why Python CI failed? I'm not familiar with the way GTSAM binds with Python.

[dellaert]

Thanks ! I'll take a look. Also wondering whether we could simply expand Cal3DS2 somehow...

[varunagrawal]

Thanks ! I'll take a look. Also wondering whether we could simply expand Cal3DS2 somehow...

I was going to suggest this. Adding the extra variable to Cal3DS2 with a sensible default would make things a lot nicer.

[dellaert]

I see the constructor issue. And there is another issue with adding k3 to, as it will change the dimension of the model. Seems like maybe the right way is to make Cal2DS2_Base templated and allow for arbitrary dimension of the k_ vector, say DimK. The Base constructor can take a p vector and k_ vector directly, while deprecating the old-style constructor. Similarly, Cal3DBase can be templated with by default DimK=2. It seems that the change to the derivatives is rather minimal, although D2dintrinsic and D2dcalibration would have to be templated and take vectors as argument.

This might be a bigger project than you had hoped for, though?

[demul]

@dellaert
I think using template and refactor the constructor with the way it takes distortion vector directly is nice idea! I didn't consider about that way before. Thanks!

This might be a bigger project than you had hoped for, though?

I think the main reason you expect this might be a big project is that,
The massive refactoring inevitably accompanied with deprecating old constructor, losing backwards compatibility.

I propose to just make a new calibration class which can takes arbitrary dimension of the k_ vector and have new style constructor on gtsam_unstable.
Then replace the old style calibration class(Cal3DS2) with that new calibration class when we migrate to 5.X API.
How do you think about it?

[demul]

@dellaert @varunagrawal
Can i get your feedback on my proposal?

I propose to just make a new calibration class which can takes arbitrary dimension of the k_ vector and have new style constructor on gtsam_unstable. Then replace the old style calibration class(Cal3DS2) with that new calibration class when we migrate to 5.X API. How do you think about it?

[talregev]

@demul I am not the maintainer, but the CI is fail on your PR.
I am suggest maybe if you can rebase your PR to the latest dev, and check why the CI is fail?

[demul]

@talregev
I'm not familiar with the way GTSAM binds with Python.
Can you tell me how to do Python CI test(Unit test of python binding of GTSAM) on my PC?

I can do C++ unit test on my PC using "make check" command.
But, how can i do Python unit test on my PC?
I've already searched any tutorials(README.md) for it on this repo, but i cannot find anything...

[talregev]

Can you tell me what os you have?
Do you have boost install?
I can write the cmak command for you with the correct flags.
Also the commands for run python tests.

[demul]

@talregev
Thanks for your help!

My OS is WSL-Ubuntu18.04
As far as i know, The build system has no significant difference with native Ubuntu18.04.
My build system is like below,
gcc-11
g++-11
cmake-3.27.6-linux-x86_64

and I installed libboost-dev/bionic,now 1.65.1.0ubuntu1 amd64

[talregev]

Thank you for your input.
I didn't ask you, but I assume you have python on your ubuntu. What version?

It seems the ci on your PR is fail due compilation error on unstable gtsam in the python script.
You can check the erros on the ci, or try to build it local on your pc.

The commands that I will write you, found here:
https://github.com/borglab/gtsam/blob/develop/.github/workflows/build-python.yml
https://github.com/borglab/gtsam/blob/develop/.github/scripts/python.sh

# if you have missing dependencies. some of them are optional. 
sudo apt-get -y install cmake build-essential pkg-config libpython3-dev python3-numpy libboost-all-dev ninja-build
sudo apt-get install -y wget libicu-dev python3-pip python3-setuptools


# install python dependencies. Assume you are on the root of the gtsam folder.
python3 -m pip install -r python/dev_requirements.txt

# run cmake command that will create a build dir. Assume you are on the root of the gtsam folder.
  cmake . \
      -B build \
      -DCMAKE_BUILD_TYPE=Debug \
      -DGTSAM_BUILD_TESTS=OFF \
      -DGTSAM_BUILD_UNSTABLE=ON \
      -DGTSAM_USE_QUATERNIONS=OFF \
      -DGTSAM_WITH_TBB=OFF \
      -DGTSAM_BUILD_EXAMPLES_ALWAYS=OFF \
      -DGTSAM_BUILD_WITH_MARCH_NATIVE=OFF \
      -DGTSAM_BUILD_PYTHON=ON \
      -DGTSAM_UNSTABLE_BUILD_PYTHON=ON \
      -DGTSAM_PYTHON_VERSION=3 \
      -DPYTHON_EXECUTABLE:FILEPATH=$(which python) \
      -DGTSAM_ALLOW_DEPRECATED_SINCE_V43=OFF \
      -DCMAKE_INSTALL_PREFIX=gtsam_install

  cmake --build build -j2

  cmake --build build --target python-install

  cmake --build build --target python-test
  cmake --build build --target python-test-unstable

You can make a PR and update the readme file for other people that have strugle to compile gtsam with python and run the test.
You can make a new PR for document if you want, and I think the maintainers will like that.

I hope this will help you.

[demul]

@talregev

Thanks for your support!
Now i'm trying to build!
but i think

-DPYTHON_EXECUTABLE:FILEPATH=$(which python) \

should become

-DPYTHON_EXECUTABLE:FILEPATH=$(which python3) \

right?

and my python version is Python 3.6.9

[demul]

@talregev

I managed to pass compilation phase by the latest commit i pushed.
but I failed at test phase.
and Cannot get any information for debug from the log(Segmentation fault) itselves...

Can you help me please...?
You can easily reproduce this error just pull my feature and run same build commands as you wrote.
Thank you!

[talregev]

@demul The compilation error on macos ci was fix.
Please rebase your code to the latest develop branch and run again the CI.
Should work.

[demul]

@talregev
I've done test on my local PC as you guided.
It works well!
Thanks ma ninja!

[demul]

@talregev
oh no... more ci failed than before. lol

[talregev]

@demul I think it just temporary fail of apt in ubuntu.
I am running your branch on my ci:
talregev#32

You should ask for maintainers to run again your PR.

[demul]

@talregev
Thanks ma ninja

@varunagrawal
Can you re-run the CI?
and can you please consider my proposal and give me your opinion about it?

I propose to just make a new calibration class which can takes arbitrary dimension of the k_ vector and have new style constructor on gtsam_unstable.
Then replace the old style calibration class(Cal3DS2) with that new calibration class when we migrate to 5.X API.
How do you think about it?

[demul]

@varunagrawal
Can you re-run CI?