Add CheckSpecies module #366
Conversation
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
There looks to be a fair bit of duplication here, can this easily be avoided?
There was a problem hiding this comment.
With a templated function and some STL magic, yes.
Co-authored-by: Dan Nixon <dan@dan-nixon.com>
| { | ||
| if (fabs(sourceParams.at(n) - refParams.at(n)) >= tolerance) | ||
| { | ||
| Messenger::print(" ... parameter {} is incorrect ({:.5e} vs. {:.5e} reference, delta = {:.5e}", n + 1, |
There was a problem hiding this comment.
Should this possibly be Messenger::error ?
There was a problem hiding this comment.
I tend to use Messenger::error exclusively for syntactic, programmatic, or otherwise 'hard' errors that should stop execution at that point. When testing, I prefer to think of these 'scientific' errors separately, and so just print normal information (partly for readability reasons when looking through the system test logs).
| #include <tuple> | ||
| #include <vector> | ||
|
|
||
| typedef std::vector<std::tuple<std::vector<int>, std::vector<double>>> IntegerDoubleVectorKeywordData; |
There was a problem hiding this comment.
Wouldn't we be better off with std::vector<std::vector<std::tuple<int, double>>? This would ensure that there are always an equal number of integers and doubles. Or is that no desireable?
There was a problem hiding this comment.
The number of integers and the number of doubles (or strings) is never guaranteed to be equal, hence the v-t-v ordering.
| typedef std::vector<std::tuple<std::vector<int>, std::vector<std::string>>> IntegerStringVectorKeywordData; | ||
|
|
||
| // Keyword with list of Tuples of Vectors | ||
| class IntegerStringVectorKeyword : public KeywordData<IntegerStringVectorKeywordData &> |
There was a problem hiding this comment.
After looking through the implementation classes, couldn't we have a single IntegerTVector template that takes the second type as a parameter? The code for the two classes seems identical. Or, if not a template, at least a base class that can get subclassed out for the parts that actually change?
There was a problem hiding this comment.
Yes, as I mentioned in the PR text this could no doubt be templated to some degree. In the first instance where there are only two similar classes I went for simplicity, but if a third (fourth...) derivative is required in the future then certainly we should template.
This PR adds a new module,
CheckSpecies, to properly assess the atom types and forcefield parameters present in a species, particularly those applied automatically from within Dissolve.Work includes:
IntegerStringVectorKeywordandIntegerDoubleVectorKeywordallowing general input of a set number of integers (in this instance, atom indices) followed by a variable list of strings or doubles. The underlying information is stored in astd::vectorofstd::tuple, with the tuple containing furtherstd::vectors of the integer and string/double data.Notes: