Fix quaternion default initialisation

[cniethammer]

This PR reverts changes to the defaults in the constructor made in 20a6b8f back to 0 + 0i + 0j + 0k (Q(w=0, x=0, y=0, z=0)) to address #291. The issue was first mentioned in #255

This has been tested using

• internal unit tests
• running examples with run-examples.sh adding the ./Generators/cubic_grid_generator/config.xml to the list of tests

[cniethammer]

Still failing the internalGenerator testcase - will need another look at this

[cniethammer]

@HomesGH Tests passing and ready for review.

[HomesGH]

To be honest, I still don't see the benefit of changing it to the invalid Q(0,0,0,0). Is it just performance-wise why we don't choose the identity Q(1,0,0,0)? There is much worse performing code, eg. the check for duplicated IDs.

For me, it would be ok to initialize the Quaternion class itself with 0,0,0,0, but all molecules should always carry valid data (Q(0,0,0,0) is invalid). Therefore, I see two options:

1. Use the identity quaternion for initialization
2. Use no defaults for the molecule class. All parameters have to be set by other classes/methods/etc. This might not be in line with the paradigm "Convention over configuration". It might also cause further trouble, see my comment below.

Furthermore, I can not guarantee that there are no hidden molecule initializations somewhere in the code. Those could break when using invalid data for initialization.

[HomesGH]

These lines could also cause trouble, even though, they should not be relevant in theory.

Also the PerCellGenerator misses the explicit passing of the quaternions.
Same here:
ParticleCellBase.cpp:154
DensityControl.cpp:374
PerCellGenerator.cpp:123

I can not guarantee that I found all Molecule/FullMolecule inits that use some of the defaults.

[cniethammer]

I address the concern about missing initialization values by changing the Molecule constructor to require all parameters to be passed - removing default values from the interface. This should catch all places during compile time!

[HomesGH]

I address the concern about missing initialization values by changing the Molecule constructor to require all parameters to be passed - removing default values from the interface. This should catch all places during compile time!

This is ok-ish for me. Even though, always having to set all of those zeros including a single 1 looks a bit error-prone to me. E.g. accidentally setting vz to 1 instead of qw is simple:
Molecule m(id, comp, rx, ry, rz, 0., 0., 0., 1., 0., 0., 0., 0., 0., 0.);
versus
Molecule m(id, comp, rx, ry, rz, 0., 0., 1., 0., 0., 0., 0., 0., 0., 0.);

[HomesGH]

@cniethammer I did a benchmark using the config.xml of the Argon example.

The second two columns are based on the master branch and just the default values in the constructor were changed.

master	Molecule init with (1,0,0,0) Quaternion init with (0,0,0,0)	Molecule init with (1,0,0,0) Quaternion init with (1,0,0,0)
34.58s	34.46s	34.89s

There is basically no difference since deviations are within uncertainties (I conducted 5 independent simulation runs each).

I would recommend reverting all the changes in this PR where the default values of the constructors were deleted and just changing the default values to be (1,0,0,0) in the molecule classes. In the quaternion constructor, we could choose either (1,0,0,0) or (0,0,0,0).

[cniethammer]

OK, I updated the PR accordingly, so Q(w=1, x=0,y=0,z=0) is the default for Quaternion and Molecule initialisation.

[HomesGH]

OK, I updated the PR accordingly, so Q(w=1, x=0,y=0,z=0) is the default for Quaternion and Molecule initialisation.

Look good! We could just fix the line breaks in the AutoPas molecule class so that all quaternions are on the same line.

[cniethammer]

That's what the current clang-format style will result in. So I would keep it as is.

If we want this kind of special formating and keep it persistent with the upcoming code reformating it will require disabling clang-format with some comments/commands... also in the other places. Let me know if I should include this as a precedential case...

[HomesGH]

That's what the current clang-format style will result in. So I would keep it as is.

If we want this kind of special formating and keep it persistent with the upcoming code reformating it will require disabling clang-format with some comments/commands... also in the other places. Let me know if I should include this as a precedential case...

I see. Than we can just keep it as it is.