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

A bunch of minor fixes #1132

Merged
merged 5 commits into from
Mar 11, 2022
Merged

A bunch of minor fixes #1132

merged 5 commits into from
Mar 11, 2022

Conversation

varunagrawal
Copy link
Collaborator

@varunagrawal varunagrawal commented Mar 11, 2022

  1. Some documentation updates.
  2. Updated print default of BetweenFactor.
  3. Removed redundant printing of body_P_sensor in PreintegrationParams.
  4. Added a Python test for ordering (Needs fixing in a later PR).
  5. Fix Scoop install in Windows CI (link).

@varunagrawal varunagrawal added the quick-review Quick and easy PR to review label Mar 11, 2022
@varunagrawal varunagrawal self-assigned this Mar 11, 2022
Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks !. Python CI seems to fail?

@varunagrawal
Copy link
Collaborator Author

Yep seems like we discovered a bug in Ordering.

@@ -88,5 +88,22 @@ def test_linearMarginalization(self):
self.assertAlmostEqual(EXPECTEDM[1], m[1], delta=1e-8)
self.assertAlmostEqual(EXPECTEDM[2], m[2], delta=1e-8)

def test_ordering(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add more comments about what is happening and what the bug is you uncovered with this? Also maybe add output so I can help think about the cause, or if you know what the problem is, let us know :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue seems to be in the way the Debug configuration is handled in the wrapper. Release versions of this same test passes no problem but we get a segfault in the Debug version. Pretty confusing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output isn't helpful, but here it is:

python/gtsam/tests/test_GaussianFactorGraph.py Fatal Python error: Segmentation fault

Current thread 0x00007fa529fd2280 (most recent call first):
  File "/home/varun/borglab/gtsam/build/python/gtsam/tests/test_GaussianFactorGraph.py", line 103 in test_ordering
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/unittest/case.py", line 633 in _callTestMethod
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/unittest/case.py", line 676 in run
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/unittest/case.py", line 736 in __call__
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/unittest.py", line 321 in runtest
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/runner.py", line 162 in pytest_runtest_call
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/callers.py", line 187 in _multicall
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/manager.py", line 84 in <lambda>
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/manager.py", line 93 in _hookexec
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/hooks.py", line 286 in __call__
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/runner.py", line 255 in <lambda>
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/runner.py", line 311 in from_call
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/runner.py", line 254 in call_runtest_hook
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/runner.py", line 215 in call_and_report
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/runner.py", line 126 in runtestprotocol
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/runner.py", line 109 in pytest_runtest_protocol
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/callers.py", line 187 in _multicall
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/manager.py", line 84 in <lambda>
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/manager.py", line 93 in _hookexec
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/hooks.py", line 286 in __call__
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/main.py", line 348 in pytest_runtestloop
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/callers.py", line 187 in _multicall
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/manager.py", line 84 in <lambda>
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/manager.py", line 93 in _hookexec
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/hooks.py", line 286 in __call__
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/main.py", line 323 in _main
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/main.py", line 269 in wrap_session
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/main.py", line 316 in pytest_cmdline_main
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/callers.py", line 187 in _multicall
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/manager.py", line 84 in <lambda>
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/manager.py", line 93 in _hookexec
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/pluggy/hooks.py", line 286 in __call__
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/config/__init__.py", line 162 in main
  File "/home/varun/.pyenv/versions/3.8.11/lib/python3.8/site-packages/_pytest/config/__init__.py", line 185 in console_main
  File "/home/varun/.pyenv/versions/3.8.11/bin/pytest", line 8 in <module>
[1]    3008430 segmentation fault (core dumped)  pytest  -s

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting out this particular test section since it is time-consuming and out of scope for this PR.

@varunagrawal
Copy link
Collaborator Author

Merging this. I'll tackle the wrapper bug later.

@varunagrawal varunagrawal merged commit 3b49d24 into develop Mar 11, 2022
@varunagrawal varunagrawal deleted the minor-fixes branch March 11, 2022 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quick-review Quick and easy PR to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants