Fix Windows Tests #1553

varunagrawal · 2023-06-21T21:36:38Z

My attempt at fixing #1541.

Add fix for compilation in Visual Studio. This seems to be a weird issue with how size_t is interpreted since the multiple symbols defined error comes from additional declarations of std::map<Key, size_t>. This might be a bug in MSVC 16.
Fixed some more tests across the repo.
Updated the Windows CI to run additional GTSAM tests as well as some gtsam_unstable tests.

There is still quite a bit of work to do to get all the tests passing on Windows, but hopefully this is a good start point.

talregev · 2023-06-29T16:08:32Z

@varunagrawal You asked my help for these tests.
I can help you to compile them, but not run them (make the tests correct).
I found some bugs that I can share with you:
1. On this line: https://github.com/borglab/gtsam/blob/develop/gtsam_unstable/CMakeLists.txt#L111
It should be GTSAM_EXPORTS instead of GTSAM_UNSTABLE_EXPORTS. There isn't definition for GTSAM_UNSTABLE_EXPORTS. I think gtsam_unstable should import/export symbol as gtsam lib.
When I switch it, it make the dll import at compilation of gtsam_unstable. Need to investigate more deeply the bug.
2. On this line: https://github.com/borglab/gtsam/blob/develop/gtsam_unstable/CMakeLists.txt#L107
It should be GTSAM_IMPORT_STATIC instead GTSAM_UNSTABLE_IMPORT_STATIC. I didn't try it.
I think it also do import instead export and need to investigate more deeply the bug.
3. On this line https://github.com/borglab/gtsam/blob/develop/gtsam/hybrid/tests/testHybridGaussianFactorGraph.cpp#L46
It should add #include <numeric> for handle the compile error: error C2039: 'iota': is not a member of 'std'

Good luck to hunt more bugs!

Edit:
After check again, I think 1 and 2 are not bug, and need to look deeper to understand why it not find symbols.

talregev · 2023-06-29T18:03:09Z

When compiling with ninja, it make the error more clear:

[24/311] Linking CXX executable bin\check_basis_program.exe
FAILED: bin/check_basis_program.exe
cmd.exe /C "cd . && "C:\Program Files\Microsoft Visual Studio\2022\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_exe --intdir=gtsam\basis\tests\CMakeFiles\check_basis_program.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100220~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100220~1.0\x64\mt.exe --manifests  -- C:\PROGRA~1\MIB055~1\2022\COMMUN~1\VC\Tools\MSVC\1435~1.322\bin\Hostx64\x64\link.exe /nologo gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testBasisFactors.cpp.obj gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testChebyshev.cpp.obj gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testChebyshev2.cpp.obj gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testFourier.cpp.obj gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testParameterMatrix.cpp.obj  /out:bin\check_basis_program.exe /implib:bin\check_basis_program.lib /pdb:bin\check_basis_program.pdb /version:0.0 /machine:x64 /debug /INCREMENTAL /subsystem:console  lib\CppUnitLiteDebug.lib  lib\gtsamDebug.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_serialization-vc140-mt-gd.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_system-vc140-mt-gd.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_filesystem-vc140-mt-gd.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_thread-vc140-mt-gd.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_atomic-vc140-mt-gd.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_date_time-vc140-mt-gd.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_regex-vc140-mt-gd.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_timer-vc140-mt-gd.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_chrono-vc140-mt-gd.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\tbb12_debug.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\tbbmalloc_debug.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\metis.lib  C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\GKlib.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cmd.exe /C "cd /D C:\src\gtsam\build\gtsam\basis\tests && C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -noprofile -executionpolicy Bypass -file C:/src/gtsam/gtsam/3rdparty/vcpkg/cache/vcpkg/scripts/buildsystems/msbuild/applocal.ps1 -targetBinary C:/src/gtsam/build/bin/check_basis_program.exe -installedDir C:/src/gtsam/gtsam/3rdparty/vcpkg/cache/vcpkg/installed/x64-windows/debug/bin -OutVariable out""
LINK Pass 1: command "C:\PROGRA~1\MIB055~1\2022\COMMUN~1\VC\Tools\MSVC\1435~1.322\bin\Hostx64\x64\link.exe /nologo gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testBasisFactors.cpp.obj gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testChebyshev.cpp.obj gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testChebyshev2.cpp.obj gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testFourier.cpp.obj gtsam\basis\tests\CMakeFiles\check_basis_program.dir\testParameterMatrix.cpp.obj /out:bin\check_basis_program.exe /implib:bin\check_basis_program.lib /pdb:bin\check_basis_program.pdb /version:0.0 /machine:x64 /debug /INCREMENTAL /subsystem:console lib\CppUnitLiteDebug.lib lib\gtsamDebug.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_serialization-vc140-mt-gd.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_system-vc140-mt-gd.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_filesystem-vc140-mt-gd.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_thread-vc140-mt-gd.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_atomic-vc140-mt-gd.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_date_time-vc140-mt-gd.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_regex-vc140-mt-gd.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_timer-vc140-mt-gd.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\boost_chrono-vc140-mt-gd.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\tbb12_debug.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\tbbmalloc_debug.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\metis.lib C:\src\gtsam\gtsam\3rdparty\vcpkg\cache\vcpkg\installed\x64-windows\debug\lib\GKlib.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:gtsam\basis\tests\CMakeFiles\check_basis_program.dir/intermediate.manifest gtsam\basis\tests\CMakeFiles\check_basis_program.dir/manifest.res" failed (exit code 1120) with the following output:
   Creating library bin\check_basis_program.lib and object bin\check_basis_program.exp
testBasisFactors.cpp.obj : error LNK2019: unresolved external symbol "public: static class gtsam::Pose2 __cdecl gtsam::Pose2::ChartAtOrigin::Retract(class Eigen::Matrix<double,3,1,0,3,1> const &,class gtsam::OptionalJacobian<3,3>)" (?Retract@ChartAtOrigin@Pose2@gtsam@@SA?AV23@AEBV?$Matrix@N$02$00$0A@$02$00@Eigen@@V?$OptionalJacobian@$02$02@3@@Z) referenced in function "public: class gtsam::Pose2 __cdecl gtsam::Basis<class gtsam::Chebyshev2>::ManifoldEvaluationFunctor<class gtsam::Pose2>::apply(class gtsam::ParameterMatrix<3> const &,class gtsam::OptionalJacobian<-1,-1>)const " (?apply@?$ManifoldEvaluationFunctor@VPose2@gtsam@@@?$Basis@VChebyshev2@gtsam@@@gtsam@@QEBA?AVPose2@3@AEBV?$ParameterMatrix@$02@3@V?$OptionalJacobian@$0?0$0?0@3@@Z)
testChebyshev2.cpp.obj : error LNK2001: unresolved external symbol "public: static class gtsam::Pose2 __cdecl gtsam::Pose2::ChartAtOrigin::Retract(class Eigen::Matrix<double,3,1,0,3,1> const &,class gtsam::OptionalJacobian<3,3>)" (?Retract@ChartAtOrigin@Pose2@gtsam@@SA?AV23@AEBV?$Matrix@N$02$00$0A@$02$00@Eigen@@V?$OptionalJacobian@$02$02@3@@Z)
bin\check_basis_program.exe : fatal error LNK1120: 1 unresolved externals

talregev · 2023-06-30T03:41:58Z

By the way, I am not sure if you notice. Even that the ci pass, some of the tests are not compile with missing symbols.

talregev · 2023-06-30T03:56:54Z

testBasisFactors.cpp.obj : error LNK2019: unresolved external symbol "class Eigen::Matrix<double,-1,-1,0,-1,-1> __cdecl gtsam::kroneckerProductIdentity(unsigned __int64,class Eigen::Matrix<double,1,-1,1,1,-1> const &)" (?kroneckerProductIdentity@gtsam@@ya?AV?$Matrix@N$0?0$0?0$0A@$0?0$0?0@Eigen@@_KAEBV?$Matrix@N$00$0?0$00$00$0?0@3@@z) referenced in function "protected: void __cdecl gtsam::Basis::VectorDerivativeFunctor::calculateJacobian(void)" (?calculateJacobian@VectorDerivativeFunctor@?$Basis@VChebyshev2@gtsam@@@gtsam@@IEAAXXZ)

varunagrawal · 2023-06-30T16:07:33Z

On this line: develop/gtsam_unstable/CMakeLists.txt#L111 It should be GTSAM_EXPORTS instead of GTSAM_UNSTABLE_EXPORTS. There isn't definition for GTSAM_UNSTABLE_EXPORTS. I think gtsam_unstable should import/export symbol as gtsam lib. When I switch it, it make the dll import at compilation of gtsam_unstable. Need to investigate more deeply the bug.

GTSAM_EXPORTS and GTSAM_UNSTABLE_EXPORTS are autogenerated as part of dllexport.h.in. So this is not a bug.

On this line: develop/gtsam_unstable/CMakeLists.txt#L107 It should be GTSAM_IMPORT_STATIC instead GTSAM_UNSTABLE_IMPORT_STATIC. I didn't try it. I think it also do import instead export and need to investigate more deeply the bug.

Same reasoning as above. :)

On this line develop/gtsam/hybrid/tests/testHybridGaussianFactorGraph.cpp#L46 It should add #include <numeric> for handle the compile error: error C2039: 'iota': is not a member of 'std'

Nice catch!

varunagrawal · 2023-06-30T16:09:04Z

By the way, I am not sure if you notice. Even that the ci pass, some of the tests are not compile with missing symbols.

Yup I am aware. That's the reason this PR is still a draft. :)

talregev · 2023-06-30T16:11:17Z

GTSAM_EXPORTS and GTSAM_UNSTABLE_EXPORTS are autogenerated as part of dllexport.h.in. So this is not a bug.

@varunagrawal Thank you the explanation. I was did a second review check, and I understand it my own that it ok. I edit my comment, and leave the original msg with strikes.

varunagrawal · 2023-08-01T14:38:01Z

@talregev @mikesheffler @dellaert I fixed all the GTSAM tests for Windows. Only the tests for GTSAM_UNSTABLE are left which I will leave for later.

talregev · 2023-08-01T15:30:43Z

@varunagrawal Are you interesting to deal with these errors?

talregev

You should squash the commits. It not recommended to have all these commits and merges to dev branch.

varunagrawal · 2023-08-01T15:33:15Z

You should squash the commits. It not recommended to have all these commits and merges to dev branch.

We prefer to keep the history so we can go back and identify bad commits and undo them. This is why we don't squash or force-push.

varunagrawal · 2023-08-01T15:34:15Z

@varunagrawal Are you interesting to deal with these errors?

Why is it using gtsamDebug.dll in the Release config?

varunagrawal · 2023-08-01T15:35:37Z

@talregev can you please approve this PR so I can merge it? The CI should pass which is a good indicator that the fixes work.

talregev · 2023-08-01T15:35:51Z

@varunagrawal Are you interesting to deal with these errors?

Why is it using gtsamDebug.dll in the Release config?

This is another bug you have in the cmake when I compile it with vcpkg.
I am not sure what happen in the regular compilation.

But the debug assertion still happen on your side. Are you interesting to solve it?

varunagrawal · 2023-08-01T15:37:20Z

This is another bug you have in the cmake when I compile it with vcpkg. I am not sure what happen in the regular compilation.

I have never had this issue. I have been compiling and debugging using the Release config for the last week and it seems to be fine. Maybe the issue is in your vcpkg settings?

But the debug assertion still happen on your side. Are you interesting to solve it?

I do not get this error on my end. I re-ran the tests again and they pass without problems.

talregev · 2023-08-01T15:38:57Z

I have never had this issue. I have been compiling and debugging using the Release config for the last week and it seems to be fine. Maybe the issue is in your vcpkg settings?

Let's deal with this later. It not for this PR.

I do not get this error on my end. I re-ran the tests again and they pass without problems.

Of course there isn't errors. It not enable. I can enable it and you will see these errors.

varunagrawal · 2023-08-01T15:41:16Z

Of course there isn't errors. It not enable. I can enable it and you will see these errors.

Sorry I don't understand. What is not enabled?

talregev · 2023-08-01T15:42:20Z

Sorry I don't understand. What is not enabled?

The flag that enable the debug assertion that enable extra checks on run time.

varunagrawal · 2023-08-01T15:43:29Z

Can you please share the changes here? I can run them locally.

talregev · 2023-08-01T15:46:41Z

I will. It will be later.

varunagrawal · 2023-08-01T15:47:09Z

I will. It will be later.

Then can you please approve this PR?

dellaert

Cool! If this is ready to merge, go for it :-)

varunagrawal added 11 commits June 20, 2023 17:35

small improvements

b0404aa

fix for multiply defined symbol error in LPInitSolver

254e312

add todo for error in SmartStereoProjectionFactorPP.h

62fe1a9

undo changes

fbf155d

use std::map<Key, uint32_t> for Key-Dim maps

5731128

Actions file formatting

429d5de

add GTSAM_EXPORT to additional Ordering methods

f65414d

mark Pose2 as GTSAM_EXPORT at the class level

43a9fbf

CameraSet::project2 SFINAE to resolve overload ambiguity

c9397c3

add GTSAM_EXPORT tags

5fa889b

enable more Windows tests

c954e54

varunagrawal requested a review from mikesheffler June 21, 2023 21:36

varunagrawal self-assigned this Jun 21, 2023

remove GTSAM_EXPORT from SignatureParser struct

0bc08b8

varunagrawal mentioned this pull request Jun 26, 2023

Add vcpkg ci #1413

Open

This was referenced Jun 30, 2023

Fix windows tests talregev/gtsam#5

Closed

Fix windows tests #1560

Closed

varunagrawal mentioned this pull request Jun 30, 2023

Fix windows tests #1562

Merged

varunagrawal and others added 5 commits June 30, 2023 15:06

Merge branch 'develop' into fix/windows-tests

a9d3a10

Compile with ninja

2168d0f

Fix linkage errors: unresolved external symbol

92de227

Add release to windows tests

a499855

Add #include <gtsam/dllexport.h>

9aa67b5

varunagrawal added 7 commits July 30, 2023 10:16

enable more tests

e562d70

fix tests in testValues

67b49d6

comment out nonlinear check

df3899e

fix nonlinear tests

ddef644

add GTSAM_EXPORT to various ostream operator overloads

ced6c53

fix SphericalCamera traits definition

c8184eb

enable slam tests

2c569c1

varunagrawal marked this pull request as ready for review August 1, 2023 14:36

varunagrawal requested a review from talregev August 1, 2023 14:37

address PR comments

8b4fbb1

talregev suggested changes Aug 1, 2023

View reviewed changes

varunagrawal requested a review from talregev August 1, 2023 16:06

talregev approved these changes Aug 1, 2023

View reviewed changes

varunagrawal requested a review from dellaert August 1, 2023 18:12

dellaert approved these changes Aug 2, 2023

View reviewed changes

varunagrawal merged commit e36590a into develop Aug 2, 2023
26 checks passed

varunagrawal deleted the fix/windows-tests branch August 2, 2023 16:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Windows Tests #1553

Fix Windows Tests #1553

varunagrawal commented Jun 21, 2023

talregev commented Jun 29, 2023 •

edited

Loading

talregev commented Jun 29, 2023

talregev commented Jun 30, 2023

talregev commented Jun 30, 2023

varunagrawal commented Jun 30, 2023

varunagrawal commented Jun 30, 2023 •

edited

Loading

talregev commented Jun 30, 2023 •

edited

Loading

varunagrawal commented Aug 1, 2023

talregev commented Aug 1, 2023

talregev left a comment

varunagrawal commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

talregev commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

talregev commented Aug 1, 2023

varunagrawal commented Aug 1, 2023 •

edited

Loading

talregev commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

talregev commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

dellaert left a comment

Fix Windows Tests #1553

Fix Windows Tests #1553

Conversation

varunagrawal commented Jun 21, 2023

talregev commented Jun 29, 2023 • edited Loading

talregev commented Jun 29, 2023

talregev commented Jun 30, 2023

talregev commented Jun 30, 2023

varunagrawal commented Jun 30, 2023

varunagrawal commented Jun 30, 2023 • edited Loading

talregev commented Jun 30, 2023 • edited Loading

varunagrawal commented Aug 1, 2023

talregev commented Aug 1, 2023

talregev left a comment

Choose a reason for hiding this comment

varunagrawal commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

talregev commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

talregev commented Aug 1, 2023

varunagrawal commented Aug 1, 2023 • edited Loading

talregev commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

talregev commented Aug 1, 2023

varunagrawal commented Aug 1, 2023

dellaert left a comment

Choose a reason for hiding this comment

talregev commented Jun 29, 2023 •

edited

Loading

varunagrawal commented Jun 30, 2023 •

edited

Loading

talregev commented Jun 30, 2023 •

edited

Loading

varunagrawal commented Aug 1, 2023 •

edited

Loading