Conversation
|
Remaining TODO (in a later PR, after I merged open PRs):
|
varunagrawal
left a comment
There was a problem hiding this comment.
I don't see anything glaring from looking at the code, but CI is failing so I can look into that.
|
PS I triggered the docker container builds manually so you are unblocked on other PRs. |
|
No, no worry about it. I'm dealing with those issues. I might close this PR if these issues are solved in a different PR. I'd rather you not add more PRs and take up limited CI resources at this point - racing against the clock. |
|
@varunagrawal I take this back. I can't figure out for the life of me why we would have errors here. We should not. ChatGPT led me down a rabbithole of gcc-specific "false positives", but clang also has issues. Since it does work on Mac, I do suspect it is somehow a difficulty with compatibility between GTSAM and GTDynamics on Linux. On my Linux machine, I have the same issues with both gcc and clang. If you can make sense of this or debug it, that would be a lifesaver. |
I already figured it out and made PR #407 targeting this branch to fix it. :) |
|
Ow, shoot, CI has the same errors I saw on my machine… could be alignment- but not issue on Mac |
|
I turned off the margin native in both repos, let's see what that does overnight. |
|
@varunagrawal hold off on edits, trying something |
Cool, I've been debugging in a docker container in the meantime. |
|
Okay, @varunagrawal, I fired up another CI. If you think you still need to make changes, please do so via PR targeting this branch and make it small. I like where I ended up after substantial editing - if it works :-) I put all the parallel makes to using |
|
I also thought I had increased memory to avoid crashes, but that seems gone too. |
|
@varunagrawal this seems to work, so I'll merge (after CI succeeds completely), and then merge master into the other PRs. If there is another urgent fix it can be made in #403 after I merge in master. |
|
Merged! Thanks for your help, @varunagrawal ! |
|
I definitely didn't touch the number of processes or the swap space recently, maybe 3 years ago? Did you manage to isolate the issue? The |
|
I'm still confused why it works in the Docker container and not on bare metal. Is it something to do with the underlying servers of GitHub actions? |
|
@varunagrawal I think you did touch it when you removed my common-ci.yml. I like it better this way, so thank you. But the number of threads was altered in the process, I think. Now everything works. We just have to make sure |
Currently the Mac and Linux CI are failing (on #403 and #404 ) because I fixed a bug on gtsam. So I would have to build new Docker images and a new brew thingy. That's very burdensome.
In the meantime, the Python CI builds the whole thing anyway.
So, this PR removes the Mac and Linux CI and expands the Python CI to also run the C++ tests. The resulting workflow, which builds gtsam for the three tested platforms, is now
common-ci.yml. I also added a new document to reflect this new situation.