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
FEEval::check_vector_compatibility: improve assert message #13420
FEEval::check_vector_compatibility: improve assert message #13420
Conversation
unsigned int | ||
get_dof_index() const; |
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.
@kronbichler I had to introduce the dof index in the FEEvaluationData
class. Is that fine? Or are there other ways to get the dof index of FEEvaluation
?
94f6fef
to
680c986
Compare
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 am not happy to add additional variables to the FEEvaluation
base class for the sole purpose of an assertion. I had rather hoped to work towards reducing the footprint of these classes given the performance-critical context in which they are used. But I think it is pretty easy to achieve what you want by letting the get_dof_handler_index()
sit in the FEEvaluationBase
class instead and implement the function there: Simply run through the matrix_free.n_components()
and let it compare pointers of dof_info
and matrix_free.get_dof_info(i)
until we find the right one (or the pointer is zero).
@@ -24,6 +24,8 @@ | |||
#include <deal.II/matrix_free/dof_info.h> | |||
#include <deal.II/matrix_free/type_traits.h> | |||
|
|||
#include <boost/algorithm/string/join.hpp> |
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.
How much code does this include file pull in?
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.
It's also a borderline use case that can easily be done differently, in case there is a huge amount of code getting in.
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.
https://www.boost.org/doc/libs/1_50_0/boost/algorithm/string/join.hpp. I have added an ifdef so that it is only included in release mode. Would that work for you?
680c986
to
6879201
Compare
I made the modification. |
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 checked and it seems we include boost/algorithm/string/join.hpp
already from before via boost/algorithm/string.hpp
, which comes in via boost/io/wtk/read.hpp
, which in turn comes in via the geometry functionality of boost, which eventually gets its way into our code via grid_tools.h
. (Boost pulls in an incredible amount of header files, for example compiling step-37 I see 625,558 lines of code in the final compilation unit after the preprocessor has finished. Probably 65% of that is boost, 20% is Trilinos, the rest is STL or our own code.)
/rebuild |
6879201
to
c4d2bb1
Compare
The assert message:
is a bit too general. In particular, the hints at the end could be improved by adding actual indices.
This PR tries to improve the assert message. For the following program:
the following assert messages are printed:
Written jointly with @mschreter.