celes_initialField: pol index doesn't update when polarization changes

[AmosEgel]

right now, the user accesses the polarization property ("TE" or "TM") and the hidden pol property is automatically set during intialization. The problem is, that if the polarization is changed later, one needs to call the setPolIdx method again. It would be good if this happens automatically.

I would suggest to change the pol property to a dependent property and rename the setPolIdx to get.pol method. Then, pol is updated when polarization is updated. @lpattelli, does this conflict with how you intended the initialization to happen?

[lpattelli]

I see your point, and I'm afraid it applies to a bunch of other properties as well. Probably, the intended way to go would be to implement a protected processTunedPropertiesImpl method as described in https://it.mathworks.com/help/matlab/matlab_prog/process-tuned-properties.html

[lpattelli]

maybe adding something like this would work?

function processTunedPropertiesImpl(obj)
    propChange = isChangedProperty(obj,'polarization') || isChangedProperty(obj,'polarAngle');
    if propChange
        validatePropertiesImpl(obj);
        setupImpl(obj);
    end
end

[AmosEgel]

So, a dependent variable would not work? I assume the above link is relevant for lookups and stuff that is expensive to evaluate. But here, this is not the case.

[lpattelli]

yes, I believe the change you proposed would work. I was just thinking of what would be a general approach to follow, since it is likely that we will find other properties that we would change, once we decide that we should not treat classes as immutable instances.

Moreover, I guess that even if this is certainly not an expensive calculation, the main overhead comes from repeatedly calling a function (possibly thousands of times in a simulation, I guess), rather than just reading a variable.

[AmosEgel]

Yet another way is a set method for polarization. It looks easier than processTunedPropertiesImpl

[lpattelli]

I might be wrong, but I think I've removed every single set method from celes, and replaced everything with the Property-Value style where all properties are set at construction time by the setupImpl method. This enforces the user to declare the properties only once at construction time, which should avoid the situation where one ends up in an inconsistent state by changing some variable at a later stage, without checking if that impacts the dependent properties. This issue proves that there was still ambiguity, though.

I know the processTunedPropertiesImpl looks like an overkill, but in the end it should be sufficient to call setupImpl (and possibly validatePropertiesImpl) inside it whenever a property is changed. I think it's just a matter of consistency/personal taste.

As an unrelated example: suppose one wants to run a series of simulations with varying wavelength. How would you proceed then? Its value impacts omega, which in turn changes k_medium and k_particle. Finding a way to re-run setupImpl on a per-class basis whenever a public property changes seems a convenient way to handle this.

[AmosEgel]

Well, I am still not familiar with the system objects. But in the "old" way you would either have omega, k_medium and k_particle as dependent variables with a get method, or you would have a set method for wavelength that updates all of them.

Note that set methods are automatically called when the user changes a property, so you cannot bypass them.

Calling the class constructor with input check each time a property is changed seems OK to me, too - if there is a way to achieve that.

[lpattelli]

I would avoid get methods as much as possible to avoid redundant calculations (I remember that functions like get.number, get.omega and get.nmax would be called hundreds of times per each iteration). On the other hand, implementing a few more set methods might work just fine. However, I think that eventually, inside each of these set methods, we would still just call validatePropertiesImpl and setupImpl to validate and update all properties in the class. Then the result would resemble very much processTunedPropertiesImpl. Or maybe we could write the set methods, each of which would perform its own validation, and then call all the set methods inside setupImpl (and get rid of the redundant validatePropertiesImpl). I used this set of functions just because they are standard methods of the matlab.System class.