Handling duplicate data during element creation

[T-Nicholls]

When we have multiple values for the same thing being passed to the element constructor how should we handle it?
We already have #695 and #894 as examples of when this causes an error, but what happens in a case where no error is caused?

>>> q1d = Quadrupole("Q1D", 0.603, 1.273, PolynomB=[0, 3.0])
>>> print(q1d.K)
3.0
>>> print(q1d.PolynomB)
[0. 3.]

So in the case of Quadrupole the value already in PolynomB is kept and the focusing strength passed as k is silently discarded.
I think this is also a problem, if the values are different then the user should at least be warned and told which value is being used.

As I see it, there are two options open to us:

1. We raise an error if the two values aren't exactly the same and neither of them is a default value.
2. We create some kind of consistent hierarchy about which value should be prioritised.

I quite like option one, after all, if we're getting two different values for an attribute something is likely very wrong with the lattice file that the user is loading from.
However, if we settle on option two, I propose the following hierarchy for how the different sources of values should be prioritised:

1. argument passed to the element constructor that is not the default value
2. attribute in kwargs that is not the default value
3. the default value

This might look something like:

class Example(Element):
    def __init__(self, default_argument=0.0, **kwargs):
        value = kwargs.pop("Attribute", 0.0)
        if default_argument != 0.0:
            if value != 0.0:
                warn("Conflicting values...etc.")
            else:    
                value = default_argument
        super().__init__(Attribute=value, **kwargs)

What are everybody's thoughts?

[lfarv]

Hi all.
#695 and #894 refer to two different problems: #894 refers to energy, which is normally not an element's property and is handled in a very specific way. See #894 (comment) for my comment on that.

#695 refers to a case where the same quantity may be accessed through different names: K/PolynomB[1], H/Polynomb[2]. I don't think there are many other examples. For this, the present rule, which is not clearly documented is:

1. keyword argument
2. positional argument
3. default value

This is different from @T-Nicholls's proposal, and the motivation is the following: if a user explicitly specifies a keyword argument after the required positional arguments, there must be a good reason. For me, his goal is to overwrite the positional argument. Moreover, in the case of a quadrupole, adding a PolynomB keyword may be necessary to introduce other multipoles, it looks to me more natural to keep it unchanged and discard the positional K value. Of course this rule can be discussed and possibly modified, with the risk of breaking the backward compatibility. #695 was corrected according to this rule. Problems appearing from now on can be considered as bugs.

However, this rule should be documented somewhere, I don't know where. In each element where the confict may happen?

Second, we have the possibility to issue a warning if a conflict is detected in the constructor. should we do it?

[T-Nicholls]

Hello @lfarv,

Apologies, I have indeed conflated two different issues; in #894 we only end up with two definitions of Energy due to a bug, whereas with PolynomB[1] & K and PolynomB[2] & H we can have multiple definitions due to intended functionality.

I am happy to go with the ordering you propose due to the reasoning you give, I would also support documenting this precedence in the docstrings of the relevant elements (only Sextupole, Quadrupole, Dipole, and Multipole I think?)

I am highly in favour of raising a warning in the constructor if there are two differing values.

[lfarv]

@T-Nicholls : I agree with both of your proposals: documentation and warnings. I'll be away for a few days, and I'll look at that when I'm back.