-
Notifications
You must be signed in to change notification settings - Fork 707
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
Split VectorTools up #9929
Split VectorTools up #9929
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the categorization. It is extermely hard to display the 13k line changes here, so I will trust you that they are split up appropriately :-)
/rebuild |
Yes, the commit history is not super clean. I can try to get every commit compiling but that's quite some more work. Otherwise, I would just squash into a few commits. |
Squashing at the end and having the full CI pass is all we need :-) |
/rebuild |
0a3a752
to
7b59637
Compare
Have you measured what this means for the overall CPU time? Many of us have many-core machines where this kind of change makes sense if you do |
I didn't change the targets. |
Oh, I see -- I hadn't paid enough attention to the fact that you're not splitting up which functions are compiled in which .cc file. |
245e844
to
f5108f6
Compare
Squashed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That gets a big thumbs up 👍 by me.
Related to #9790. This also allows to only include smaller headers than all of
VectorTools
but I didn't make that transition for the examples or the tests.The approach taken here, was to first split up the
template
file using the instantiations files and then splittingvector_tools.h
matching the template files. If we don't want to split upvector_tools.h
it's easy enough to revert that.The actual changes are pretty small, it's mostly only moving code around.
Some things that changed:
dim<spacedim
for the boundary functions. We were previously generating them implicitly through theproject
functions.I verified manually that the documentation didn't change at first sight.