Fix cosine angle generator constructor

[richeldichel]

The default constructor did not include a default value for the distribution that should be used. This meant that users needed to enter a distribution or otherwise the generated angle would just be 0. Further, the copy constructor always used the classic distribution. Both things have been fixed with this commit.

This pull request also includes an option to change the direction. As the angle can only take values between 0° and 90°, generation could only take place in one direction.
Therefore, the generator now includes a direction parameter which specifies whether the direction is in forward (0° <= angle <= 90°) or backward direction (180° >= angle >= 90°).

[Copilot]

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

Kassiopeia/Generators/Source/KSGenValueAngleCosine.cxx:13

• The copy constructor does not copy the newly added members fMode and fDirection from the source instance. Consider updating the copy constructor to initialize these members from aCopy to ensure consistent behavior.

KSGenValueAngleCosine::KSGenValueAngleCosine(const KSGenValueAngleCosine& aCopy) :

[Copilot]

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

Kassiopeia/Generators/Source/KSGenValueAngleCosine.cxx:50

• Consider adding unit tests for the backward direction logic to verify that the angle is correctly transformed.

if (fDirection == EDirection::Backward) {

[2xB]

Looks good to me! I just wonder if it makes sense to not use IContains but to actually compare strings so there is no ambiguity and it is more straightforward to add new configuration options with similar names in the future?

[richeldichel]

Looks good to me! I just wonder if it makes sense to not use IContains but to actually compare strings so there is no ambiguity and it is more straightforward to add new configuration options with similar names in the future?

I basically followed the implementation design of many other bindings, where IContain is also used. In my view, this is also a good approach because the user does not need to worry about capitalization or whether to write 'backward' or 'backwards'. I also don't expect new configuration options to appear for this, because there are only two directions in which a particle can start with angles between 0 to 90° :)
So, I would just keep it as is.

[2xB]

I'd also be fine with KBaseStringUtils::IEquals. But I don't think it is good style to use IContains, independent of whether this is used for other configuration options.

[2xB]

As discussed, I take my above comment back since for the "mode" option, I believe we should not alter existing configurations too much and for the new option "forward" and "forwards" both make sense, so there is an argument to be had for allowing both. In any case, LGTM!