Skip to content

Eigen submodule#3320

Merged
roystgnr merged 5 commits intolibMesh:develfrom
roystgnr:eigen_submodule
Jun 30, 2022
Merged

Eigen submodule#3320
roystgnr merged 5 commits intolibMesh:develfrom
roystgnr:eigen_submodule

Conversation

@roystgnr
Copy link
Copy Markdown
Member

Closes #3244 (if it works for others too) and obviates #3245. Our make check and MOOSE run_tests seem happy enough with it on my system, but let's see what CI thinks.

I'd like to remove 3.3.9 now that we have 3.4.0 via the new submodule, but I notice we still even have 3.2.9 hanging around, and perhaps there's a reason for that.

@roystgnr
Copy link
Copy Markdown
Member Author

One bit of difference between this and our other submodules: the host for Eigen is Gitlab, not Github, so there was no using relative URLs for it. I chose the https over the ssh URL in the hopes that that would be compatible for more people (even if less flexible for some).

That doesn't seem to explain why CI isn't finding the headers and my builds are. I'll try a fresh checkout and see if I can replicate.

@roystgnr
Copy link
Copy Markdown
Member Author

Well, that's a bit more serious. Eigen 3.4.0 uses std::invoke_result, but Clang 5's libc++ doesn't provide it.

@loganharbour
Copy link
Copy Markdown
Member

Time to upgrade clang min?

@moosebuild
Copy link
Copy Markdown

moosebuild commented Jun 16, 2022

Job Coverage on 75c29b9 wanted to post the following:

Coverage

148032 #3320 75c29b
Total Total +/- New
Rate 55.73% 55.73% -0.00% -
Hits 44903 44901 -2 0
Misses 35672 35674 +2 0

Diff coverage report

Full coverage report

This comment will be updated on new commits.

@roystgnr
Copy link
Copy Markdown
Member Author

Time to upgrade clang min?

It's astonishingly hard to get any idea.

Searching for "libc++ invoke_result" leads to a mailing list entry with a broken link to https://libcxx.llvm.org/cxx1z_status.html ; https://libcxx.llvm.org/cxx17_status.html isn't there either.

https://releases.llvm.org/12.0.0/projects/libcxx/docs/Cxx1zStatus.html is a thing, but it doesn't seem to exist for 11.0.0 or earlier.

https://releases.llvm.org/13.0.0/projects/libcxx/docs/Status/Cxx17.html seems complete enough, but it only lists "First released version" numbers for "Papers", whereas for "Issues" (like 2807, which seems to be the most related to our error here) it only lists "Complete" or not. (and of course that URL has no equivalents for 12.0.0 or earlier)

I don't have anything older than clang++-11 locally anymore, and even if I did it wouldn't matter because Ubuntu clang is configured to use libstdc++ rather than libc++.

moosebuild shell centos-7 has our clang/5.0.2 but nothing newer.

moosebuild has clang/9.0.1 available, and it seems to be using its own libc++; that would be a start ...

Or I can dig through their git history. Okay, finally I had a smart idea. It looks like llvm/llvm-project@bcde6e7 added invoke_result on 2017-12-12, and that made it into the 6.0.0 release. So we wouldn't actually have to increase our min clang very far at all. I'd be up for that.

roystgnr added 3 commits June 29, 2022 17:24
That's the newest release; it's several months old but it at least has
the fix that we had to backport to 3.3.9
@roystgnr
Copy link
Copy Markdown
Member Author

Looks like we're happy, post-clang-upgrade.

@jwpeterson
Copy link
Copy Markdown
Member

Quick thought: what happens with make dist now that Eigen is an (optional) submodule? With our other (required) submodules, we basically just don't let you run make dist unless they are initialized. I suppose I could look through the diff more closely and find out... but this seemed easier :)

@roystgnr
Copy link
Copy Markdown
Member Author

I suppose I could look through the diff more closely and find out...

Yes, yes, that would be a good idea. Later ... After I've pushed some ... completely unrelated and unimportant changes ...

But seriously, thanks for catching that. I'll add something along the lines of what we've got for MetaPhysicL, I'll do the same for poly2tri while I'm at it, and I'll try out "make dist" after unloading each.

roystgnr added 2 commits June 30, 2022 09:47
Also give more specific error messages if there's a submodule missing
@roystgnr roystgnr merged commit d9746fb into libMesh:devel Jun 30, 2022
@roystgnr roystgnr deleted the eigen_submodule branch June 30, 2022 17:06
@pbauman
Copy link
Copy Markdown
Member

pbauman commented Aug 20, 2022

I was building master for the first time since this was merged (way too long... :( ) and I ran into a issue. My build script has git submodule update --init --recursive (which is also what is currently documented on the README), but it seems that was not enough to pull in these eigen changes. However, doing a fresh clone (again followed by git submodule update --init --recursive) seems to have done the trick. Do I need to add another option to the git submodule command? Either way, wanted to document for anyone else that may run into this that at least a fresh clone (even if a less than ideal solution) resolved the issue for me.

@boyceg
Copy link
Copy Markdown
Contributor

boyceg commented Aug 20, 2022

I was building master for the first time since this was merged (way too long... :( ) and I ran into a issue. My build script has git submodule update --init --recursive (which is also what is currently documented on the README), but it seems that was not enough to pull in these eigen changes. However, doing a fresh clone (again followed by git submodule update --init --recursive) seems to have done the trick. Do I need to add another option to the git submodule command? Either way, wanted to document for anyone else that may run into this that at least a fresh clone (even if a less than ideal solution) resolved the issue for me.

I almost never am able to get the submodule update to work, and then seem to wind up in a state where I have to do a fresh clone of the full repo.

@jwpeterson
Copy link
Copy Markdown
Member

Do I need to add another option to the git submodule command?

There are no other options required as far as I know. I have multiple repos on different machines and the switchover to the Eigen submodule went fine for all of them. There is one command, git submodule sync --recursive, that IIRC needs to be run whenever there is a change to the URLs in the .gitmodules file (I could be wrong about exactly when sync needs to happen). Whenever something goes wrong with submodules, this is the one thing that I usually try.

@jwpeterson
Copy link
Copy Markdown
Member

I almost never am able to get the submodule update to work, and then seem to wind up in a state where I have to do a fresh clone of the full repo.

Are you on an unreliable network by any chance? I cannot recall having any issues with submodules that forced me to make a fresh clone, but I haven't had a download fail in the middle of fetching, either. The switch to the eigen submodule requires a relatively large initial download, so the chances of a failure in the middle of that seem like they'd be greater.

@pbauman
Copy link
Copy Markdown
Member

pbauman commented Aug 22, 2022

git submodule sync --recursive

Thanks, I'll try and remember that one

Are you on an unreliable network by any chance?

No this is a pretty reliable network and there were no interruptions when I ran my build script (which does the git submodule command before a git pull and then builds)

@jwpeterson
Copy link
Copy Markdown
Member

jwpeterson commented Aug 23, 2022

No this is a pretty reliable network

OK, that question was mainly for @boyceg, just curious about the nature of his issues with submodules since I have not experienced what he described.

@pbauman
Copy link
Copy Markdown
Member

pbauman commented Aug 23, 2022

Oh, sorry, I didn't read carefully to whom you were replying. :(

@boyceg
Copy link
Copy Markdown
Contributor

boyceg commented Aug 29, 2022

No this is a pretty reliable network

OK, that question was mainly for @boyceg, just curious about the nature of his issues with submodules since I have not experienced what he described.

I am almost surely just guessing the wrong git submodule command. (IIRC, my experience is that git submodule update --init --recursive only works for the initial checkout.) What is the right way to do this? 😁

@jwpeterson
Copy link
Copy Markdown
Member

git submodule update --init --recursive

is what I type every time I pull and notice there are changes to the submodules.

@boyceg
Copy link
Copy Markdown
Contributor

boyceg commented Aug 29, 2022

Hrmmm, I must be doing something else wrong.

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.

Update libEigen (deprecated std::result_of)

6 participants