-
Notifications
You must be signed in to change notification settings - Fork 774
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
Unable to Build 4.1.1 On Windows #1087
Comments
Out of curiosity, could you try building GTSAM with the VS Clang instead of MSVC? MSVC is very foreign to the entire team, and I've heard a lot of good comments from friends using Clang. |
I will try that right now and get back to you. |
Looked back at it and I believe I ran into the same problems as before despite using Clang instead of MSVC. Is there anything else I should try out? Perhaps I am not doing something right within VS? |
Hi, I think what we test in CI and currently is MSVC. @ProfFan, could you possibly help with debugging that? That would then help everyone if there actually is an issue. |
I'd be happy to hop on Zoom to take a look if that would make things easier. Otherwise, I am more than happy to provide as much info here as you need. Thanks! |
Just wanted to bump this issue to see if anything has been discovered. If not, no worries! I understand how busy everyone must be! |
Yeah, not only busy, but the core issue is most of us are on Linux or Mac. I tried compiling on an old windows laptop but could not get past boost library issues. It's odd, as our CI passed for 4.1.1., right? Can you look at the CI workflow and see what might be different on your system? If nothing jumps out, I have this: a lot of the warnings in your original post concern timing scripts. How about trying a simpler target, e.g. And, finally, did you try latest develop? Sometimes a missing GTSAM_EXPORT is what is needed, so if develop works and we see what GTSAM_EXPORTs have been added since 4.1.1, we can do a patch 4.1.2 to make it compile on windows... |
I saw this issue pop up, so I thought I'd give it a whirl as well. It looks like I am also having issues. I'm also pretty busy, but I'd like to help out with building and trying out fixes, if they aren't crazy-involved.
Looking at what I believe to be the Windows CI Configuration, I matched up the settings as closely as I could (Visual Studio 2019, but Community instead of Enterprise; pre-built boost, but 1.73 instead of 1.72, etc.). Ultimately, I want
I used the latest I was able to build
In the Build ErrorsA looooot of build errors. Looks like most of them are probably
Complete list of errors:
|
Wow, thanks @mikesheffler ! Two things:
However I just looked at the first file, It's telling that the windows CI does not build any unstable targets. Again, latest edits by @varunagrawal in the workflow, so he might be able to tell us more. |
Yeah I ran into this issue a ton when trying to speedup the windows CI. The unstable build was never turned on which is why this issue wasn't caught before. I tried adding GTSAM_UNSTABLE_EXPORT to the complaining classes and they didn't seem to do anything? At that point I sort of gave up because I had more pressing things to attend to and the Windows dev skill needed was above my pay-grade. We may have to ask the folks at Robostack for help at this since they have the needed expertise. |
I don't actually know how to read the CI configurations, so I'm going to take your words for it, but looking at this action, I thought that that meant unstable was being built. There are a ton of references in there that seem to suggest that it is. I created a pull request (#1097) that fixes the issues in However, back in
One part of this that I don't know how to untangle is that the definition for class GTSAM_UNSTABLE_EXPORT LocalOrientedPlane3Factor
: public NoiseModelFactor3<Pose3, Pose3, OrientedPlane3> and template<class VALUE1, class VALUE2, class VALUE3>
class GTSAM_EXPORT NoiseModelFactor3: public NoiseModelFactor are we seeing shenanigans relating to a I'm willing to look at the MATLAB build (w/o |
Yeah, that is weird. I think abstract classes like NoiseModelFactor3 should not have EXPORT, actually. Could you see whether removing that works? |
Removing After making those local changes, I tried tackling the link errors associated with Throwing a dart at the rest of the list, I looked at
and I noticed the same structure mentioned before where I still don't know what happens when we have a
Sure! Does that mean adding |
Yeah, I don't know either. I tried re-reading https://github.com/borglab/gtsam/blob/develop/Using-GTSAM-EXPORT.md and it seems to say that as long as a class has at least one function defined in a .cpp, it should be OK to do GTSAM_EXPORT. FWIW I think mixing GTSAM_EXPORT and GTSAM_UNSTABLE_EXPORT is not an issue, and they probably are About #pragma: most warnings I see seem related to BOOST_CONCEPT_CHECK, which is not used in many places, so maybe that's the way to go, yes. Finally: you can add some unstable checks in the CI script (at the very end) to make sure things actually work |
tl;dr I think I got it. The key seemed to be removing some export statements. I can put together a PR later tonight, once I try to quiet some of the warnings, and learn how to run the tests that @dellaert asked for.
Aaaaaaaaaah. I was not aware of that. It makes sense, but I didn't think of it at all. Would you like to guess what the genealogies of all of the complaining classes have in common? There was a
That sounds plausible to me, but that wouldn't (I don't think) explain why I had to change class GTSAM_EXPORT Constraint : public DiscreteFactor to class GTSAM_UNSTABLE_EXPORT Constraint : public DiscreteFactor in my previous PR.
Okay, I will try to add some #pragma statements to a few files and try to get the compiler to settle down a little with the warnings.
Sorry for asking about such a basic issue, but I don't actually know how to do this. Looking at the Windows build Actions, I see what was run for recent commits (I think). I don't know what to modify to run the tests you are asking for. Could you please explain (or, if there's something that I can read, that works too)? |
Woah, good sleuthing :-) about the last point, I’m not an expert either on these workflows :-) however, at the end of this file: https://github.com/borglab/gtsam/blob/develop/.github/workflows/build-windows.yml you can add any target that is defined in GTSam. You can do |
There are some more details in #1102 , but I had to cave on silencing the warnings and adding checks.
Here's the list from my notes: Passcheck.base Failcheck.basis I have not looked into why any of these are failing. Many of them might be straightforward to fix, but I don't know, at present. |
Getting back to the original question of getting the MATLAB toolbox building, here is the current build output, for posterity. This is just the 'regular' toolbox, and not attempting to add The formatting here is really frustrating, and many of the lines are 'Hint on symbols that are defined and could potentially match` 🙄 , but I don't really know a better way to present it. Build Errors for
|
That's weird. I know @mattking-smith uses the MATLAB toolbox, so he must have gotten that to compile. Matt? |
Trying to resolve #91 and #1102, ultimately getting MATLAB and python wrappers working on windows. Borrowing from your little bird analogy, I propose using make check as the canary in the coal mine: this would suss out all compile and linking errors in gtsam proper, make our windows CI better, and move on to any issues in building the MATLAB wrapper. Makes sense? |
@dellaert I am using the MATLAB toolbox in a Ubuntu 20.04 environment and have everything successfully compiling, wrapping, and running with no issues. So this may be a Windows compilation issue? Are these MATLAB toolbox issue really related to the toolbox itself or an OS compilation problem? Gauging from #1087 (comment) it looks like other non-MATLAB toolbox things are going on here. |
Aloha! I was attempting to build gtsam as well and ran into this issue. I believe the issue lies with exporting templates and their specializations. I'm currently using VS2022 on the v142 toolset. Here is a tiny example of one of the linkage errors - only because it was the first link error that popped up while compiling. I will be focusing on the timing/timeShonanFactor application.
To fix these two link errors, the template function specializations need to be advertised in gtsam/slam/dataset.cpp by adding GTSAM_EXPORT.
Alternatively, these type of unresolved symbol link errors go away if you disable BUILD_SHARED_LIBS in the cmake configuration step. |
@oicchris Thank you! Can you put up a small PR for that fix? |
Oof, I apologize that I do not have time to fix all of the export symbols. Embarrassingly, I went with the alternate static library route. |
We've massively improved the Windows compilation and a lot of the linker issues have been resolved. I guess we can close this issue unless there is something else that needs to be addressed. |
Description
I have been trying to compile GTSAM from source on at least 3 different Windows 10 machines but keep running into problems. My goal is to compile the most recent stable version (4.1.1 at the time of writing) in release and generate the MATLAB toolbox alongside it. I am first trying to just get release to successfully compile before playing with the MATLAB build so that I can hopefully isolate problems. Unfortunately, since the compile time takes so long (2-3 hours at least), I can't quickly try a new configuration to see if it works. For now I am getting by with a precompiled GTSAM MATLAB toolbox I found somewhere (can't recall where) which is labeled as version 3.2.0. However, I would like to get it working with the most recent version and especially with the release version instead of debug for speed.
Steps to reproduce
bootstrap.bat
followed byb2.exe
Where is the source code: C:/Users/<USERNAME>/Desktop/GTSAM/gtsam-4.1.1
). Create abuild
directory inside thegtsam-4.1.1
directory and point cmake to that for building (i.e.,Where to build the binaries: C:/Users/<USERNAME>/Desktop/GTSAM/gtsam-4.1.1/build
).BOOST_ROOT
to point to the unzipped and precompiled Boost directory (i.e.,Add Entry
->Name: BOOST_ROOT
,Type: PATH
,Value: C:/Users/<USERNAME>/Desktop/boost_1_78_0
).GTSAM_WITH_TBB
tooff
to suppress it):Generating done
.Open Project
, next to the button it saysCurrent Generator: Visual Studio 17 2022
.debug
torelease
. Then clickBuild
->Build Solution
.Below are the first few error codes Visual Studio provided for me. I am omitting the majority of them for space but would be happy to provide more information upon request.
Expected behavior
I expected GTSAM to compile properly.
Environment
Running on Windows 10 with an AMD Ryzen Threadripper 3990X and 256 GB of RAM (I have also tried these same steps on a much less powerful home computer also running Windows 10). Using the GUI version of cmake and Visual Studio 2022 (used Visual Studio 2019 on the home computer).
GTSAM Version: 4.1.1 (downloaded from the releases section on GitHub)
Boost Version: 1.78.0 (I have tried both Boost from source and also the prebuilt windows binaries link provided on the Boost website)
Additional information
N/A
The text was updated successfully, but these errors were encountered: