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

Compatibility issues with GTSAM v4.2 fixed #7

Closed
wants to merge 9 commits into from

Conversation

AmirSamanMirjalili
Copy link

Dear Developer

firstly, nice job on your accomplishment on developing this dynamic factor graph.
i was playing with your source and faced some couple of issues that breaks your code with GTSAM 4.2 structure.
i will be honored if you check the fixes plus some notes in README.md file
also i have another question about the implementation itself. the simulation runs well till t=50s for FourBar mechanism then the constrains start to break. do you have any clue why?

Warm Regards
Amir Saman Mirjalili

@jlblancoc
Copy link
Member

Hi @AmirSamanMirjalili !

Thanks a lot for your interest in the project and your contribution.

I'm closing this PR since I needed to add some clarifications in the original README, so let me explain a couple of proposed changes so you can open a new PR:

  1. As it is now explained in the README, the code was designed to work with the latest GTSAM version (>=4.3), but since 1+ years passed and it's now released yet, let's make the default branch "master" compatible with GTSAM 4.2 (most of your changes), leaving the original code in another branch for the newer versions to come.

  2. I think you deleted a note on a "TO DO" related to TBB. That's still a real issue: GTSAM must be built WITHOUT TBB, or the factor graphs will not be evaluated right. There's a need for a significant refactoring before getting rid of that... But at least, leave the comment with the warning! :-)

  3. Please, place your example mechanism under config/mechanism for coherence with the rest of examples (??).

  4. Please, pass clang-format-14 to your copy and commit the changes to ensure a consistent formatting. See my note at the end of the new version of the README.md.

Cheers!

@jlblancoc jlblancoc closed this May 22, 2024
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

Successfully merging this pull request may close these issues.

2 participants