I would like to add some new code to the “math_extra.h” file. I realize this will be met with some resistance, but we can debate the merits for this in a different thread.
The question I would like to ask is:
Is it okay to include modern C++ features in this file?
At a minimum I will need to use templates and std::vector.
Optionally, I would like to also use std::complex and lambda-expressions (which is a C++11 feature). (This will require gcc 4.8 or later, or clang 3.3 or later.)
Will this be okay? I posted this on the mailing list in case other developers have the same question.
Thank you very much in advance for your consideration.
Andrew
P.S. TLDR (feel free to skip the next part…)
If you’re curious, the lambda-expressions are necessary to add a particular matrix diagonalization library which can work with large and/or sparse matrices in a relative general and elegant way. I realize that there may not be a need for this in LAMMPS yet, but since I am completely rewriting the eigenvector code in LAMMPS, I thought might as well add this useful feature. Incidentally, we cannot continue with the existing matrix diagonalization code since it only works for 3x3 matrices and it violates the Numerical Recipes license. (And as it happens, my replacement code turned out to be a little bit faster.) As for me, several features Jacob and I are adding to fix bond/react require the ability to diagonalize 4x4 matrices. However there are so many more potential ways this new general matrix code could be useful beyond what we are doing.
I confess I have personally known several cluster sysadmins who were unwilling to upgrade their OS from over 10 years ago. It’s annoying. (But they were willing to install new whatever standard libraries are needed to get LAMMPS to compile on these old machines.)
Please let me know if you have any questions or concerns. Thanks again.
I would like to add some new code to the “math_extra.h” file. I realize this will be met with some resistance, but we can debate the merits for this in a different thread.
The question I would like to ask is:
Is it okay to include modern C++ features in this file?
At a minimum I will need to use templates and std::vector.
Hmm… I can live without std::vector (and even templates) if I have to.
(TLDR: I just remembered that LAMMPS has its own version of malloc/free, and I don’t want to interfere with that. If I did use std::vector, it would only be to store individual eigenvectors, not entire matrices. So hopefully those vectors would not require more than a negligible amount of space compared to the other things LAMMPS is keeping track of, like atom coordinates. But if you want me to avoid std::vector, I can definitely do that.)
unless something drastic happens that will change our minds, I expect that we will make C++11 a requirement for all of LAMMPS after the upcoming stable release in a few weeks.
Regardless of that, the requirement for any changes to the core code of LAMMPS is that it is clean, readable, somewhat minimalist, with as little side effects as possible and required to be placed there or useful to multiple use cases.
The last point is something where sometimes different opinions differ, and it cannot be really said if a change would be accepted unless there is a suitable proposal and a demonstration where it can be used. Templates are often a mixed blessing as they can lead to confusing compiler error messages and require to make all code inline if not explicitly instantiated, and thus can lead to significant code bloat. They also can be used for elegantly writing generic and efficient code and hiding nasty technical details from “normal” programmers. So, again, it depends on the specific case and amount of code, whether this would be a welcome change or not.
All that said, there is nothing from implementing what you need in the (optional) package code first and then submit a proposal to move things into the core code and adapt other users of the same functionality to use the new code (and demonstrate the benefits).
Don’t worry much about the internal malloc. This is mostly for multi-dimensional arrays. There are by now lots of cases, where 1d arrays are handled with new/delete.
unless something drastic happens that will change our minds, I expect
that we will make C++11 a requirement for all of LAMMPS after the
upcoming stable release in a few weeks.
I’m glad to hear this.
Regardless of that, the requirement for any changes to the core code of LAMMPS
is that it is clean, readable, somewhat minimalist, with as little side effects as
possible and required to be placed there or useful to multiple use cases.
The last point is something where sometimes different opinions differ, and it cannot
be really said if a change would be accepted unless there is a suitable proposal and
a demonstration where it can be used.
That’s fair.
Templates are often a mixed blessing as they can lead to confusing compiler
error messages and require to make all code inline if not explicitly instantiated,
and thus can lead to significant code bloat. They also can be used for elegantly
writing generic and efficient code and hiding nasty technical details from “normal”
programmers. So, again, it depends on the specific case and amount of code,
whether this would be a welcome change or not.
All that said, there is nothing from implementing what you need in the (optional)
package code first and then submit a proposal to move things into the core
code and adapt other users of the same functionality to use the new code
(and demonstrate the benefits).
That sounds fine to me.
Comment: One of the benefits is a legal one. (We should gradually move away from relying on Numerical Recipes.) But we can postpone the conversation about where to put this new code until Jacob and I demonstrate a use case.
Don’t worry much about the internal malloc. This is mostly for multi-dimensional
arrays. There are by now lots of cases, where 1d arrays are handled with new/delete.
Great. But I’ll refrain from using std::vector unless it makes the code easier to read and doesn’t waste memory or slow things down measurably (eg. due to bloat).
Axel.
Thanks for addressing my concerns. I can proceed now.