Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary null pointer checks #4

Closed
elfring opened this issue Mar 23, 2020 · 10 comments
Closed

Remove unnecessary null pointer checks #4

elfring opened this issue Mar 23, 2020 · 10 comments

Comments

@elfring
Copy link

elfring commented Mar 23, 2020

Extra null pointer checks are not needed in functions like the following.

@GSORF
Copy link
Owner

GSORF commented Mar 23, 2020

Extra null pointer checks are not needed in functions like the following.

* [AccumulatedSCHessianSSE](https://github.com/GSORF/Visual-GPS-SLAM/blob/7979f3905f17993d85063171d42b4d29e7eb5da9/03_Application/dso/src/OptimizationBackend/AccumulatedSCHessian.h#L55)

* [EnergyFunctional::setAdjointsF](https://github.com/GSORF/Visual-GPS-SLAM/blob/7979f3905f17993d85063171d42b4d29e7eb5da9/03_Application/dso/src/OptimizationBackend/EnergyFunctional.cpp#L46)

* [PhotometricUndistorter](https://github.com/GSORF/Visual-GPS-SLAM/blob/7979f3905f17993d85063171d42b4d29e7eb5da9/03_Application/dso/src/util/Undistort.cpp#L131)

Hello @elfring ,

Thank you very much for your contribution, it is much appreciated! Actually, the code problems you have linked, exist in the open source algorithm which was used by this project. It is called Direct Sparse Odometry and their repository can be found here: https://github.com/GSORF/Visual-GPS-SLAM#dependencies
However, there is actually a newer version available, called the DSO with Loop Closure (LDSO) here that has a complete restructuring of the source code and therefore these bugs might not be there anymore: https://github.com/tum-vision/LDSO

So, do you agree to close your issue here?

Thank you and kind regards,
Adam

@elfring
Copy link
Author

elfring commented Mar 23, 2020

Do you care for further improvements in related software areas?

@GSORF
Copy link
Owner

GSORF commented Mar 23, 2020

Do you care for further improvements in related software areas?

Yes!

@elfring
Copy link
Author

elfring commented Mar 23, 2020

Under which circumstances will adjusted software components be integrated here?

@GSORF
Copy link
Owner

GSORF commented Mar 23, 2020

Under which circumstances will adjusted software components be integrated here?

This project is not so much about using the DSO specifically. It may be used with any Visual SLAM software because all the sensor data fusion is done in a Blender / Python implementation. The DSO was just used as an example to generate the trajectories for the observations in the Kalman filter.

And as the dependencies are not included as git submodules, they need to be copied over here manually from their original repository. This is why the dependencies will not be updated in this repository.

Thank you for your comment.

Kind regards,
Adam

@elfring
Copy link
Author

elfring commented Mar 24, 2020

🔮 Can it become easier to identify and improve bundled software components?

@GSORF
Copy link
Owner

GSORF commented Mar 24, 2020

🔮 Can it become easier to identify and improve bundled software components?

In general: Sure, but I don't know how.
For this project, however, I can add more detailed instructions to the README, additionally to what I have already, which clearly state what I have modified and therefore what other users would need to change to the original codebase. Maybe I could even provide a diff-file for git which applies those changes. Is this what you have in mind?

Kind regards,
Adam

@elfring
Copy link
Author

elfring commented Mar 24, 2020

I suggest to stress the distinction between own and external software.
I see a few possibilities for this purpose.

  • Extend documentation.
  • Adjust the directory hierarchy.

Will the corresponding clarification trigger any collateral evolution then?

@GSORF
Copy link
Owner

GSORF commented Mar 26, 2020

Thank you for your suggestions, I will extend the documentation to this regard.
As already mentioned, the point of this project is not the DSO, it was just used to generate the trajectories which are used in the Fusion code which is own software. the changes to its codebase were required because it did not output them in the original version.

It is obviously not clear enough from the README, I will therefore improve it on this weekend and then close this issue.

Thank you, kind regards,
Adam

@GSORF
Copy link
Owner

GSORF commented Apr 24, 2020

I have now updated the README file and described my modifications in more detail, see here:
https://github.com/GSORF/Visual-GPS-SLAM#modifications-to-the-original-code-base-of-direct-sparse-odometry-dso

Therefore I will now close this issue. Thank you for your suggestion.

@GSORF GSORF closed this as completed Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants