WIP: allow model parameters to be passed into model element constructors
This changes the way model parameters are created and added to model elements to allow !39 (closed) to work.
Model parameter objects for an element are currently created in __new__
and assigned default values before __init__
gets called. Then any values passed into __init__
for these parameters (R
, T
, etc.) are set in the associated model parameter object. There is no way to set an element's model parameter object directly, but this is something that will be required by the parser in order to support self-referencing parameters like m m1 R=1-&m1.T T=0.01
. The changes required to support this (see !39 (closed)) involve building Parameter
objects before their associated model elements, and this is what this MR changes.
Now model parameters are created during ModelElement.__init__
, not __new__
. This has wide-ranging effects on code that previously relied on the old behaviour, noted below.
Noteworthy changes
Where model parameters now get created
Model parameters used to be created in __new__
, but are now created in ModelElement.__init__
. This gives child classes the opportunity to set a model parameter object directly, and thus allows the parser to pass such pre-built parameters as arguments. If a built Parameter
is not passed, the same behaviour as before is used: a new Parameter
is built and assigned either the passed value (if it's a float or whatever) or the default.
Parameter
constructor
Owning elements are no longer set in the Parameter
currently needs its owning component passed into its constructor. This MR changes it to not take the owner during its own __init__
, but rather to have it be assignable as a property. The owning component is not currently required in __init__
as far as I can tell; it's only needed later, e.g. to get the parameter's name (for repr
etc.) and its associated model.
__init__
Setting model parameter values in Element constructors now need changed to pass their model parameters to ModelElement.__init__
. Currently they usually just set their own fields directly, like self.phi = phi
, but this bypasses the checks in ModelElement.__init__
to determine if the passed value is actually an already-built Parameter
as would be the case with the parser. The constructors for all public elements therefore need changed to forward the parameter values passed into the constructor up to ModelElement.__init__
.
Default values for parameters
Currently the model parameters had two defaults: the default specified in the @model_parameter
decorator, and the default in the __init__
signature. The first was being overridden by the second in most cases, making the default defined in the model parameter mostly useless. Now model parameters don't have defaults defined any more, and are always passed their value one way or the other by ModelElement.__init__
, either using the default specified in the concrete class's constructor signature or using the value specified by the user.
Surface
class
The Surface
class, which implements common functionality between Mirror
and Beamsplitter
, no longer needs @model_parameter
decorators of its own - these were never used, nor useful, since they had to be defined on Mirror
and Beamsplitter
anyway.
Things that will break and need fixed
Mirror mass and signal gain
Mirror
previously didn't have mass
and signal_gain
@model_parameter
decorators, but did support these arguments in the signature. When I added support for these arguments as decorators, I got an error with the modal simulation code - I think it doesn't handle these parameters properly, even though it accepted them. I therefore removed them for now but we should probably add them back.
Model elements can't inherit from ABC
For some reason, if a model element child class (e.g. Detector
) has an ABCMeta
metaclass, or is a direct subclass of ABC
, it breaks some Cython type definition:
This has been fixed.
________________________________________________ ERROR at setup of test_propagation__frequency_offset[3141] ________________________________________________
model = <finesse.model.Model object at 0x7fa64aa29670>
@pytest.fixture
def laser_and_mirror_with_ad(model):
"""Model with a laser and mirror separated by a space, with amplitude detector."""
model.chain(Laser("L0"), Space("s0"), Mirror("M", R=1, T=0))
> model.add(AmplitudeDetector("ad_lsr", model.L0.p1.o, model.L0.f.ref))
tests/functional/test_space_phase_propagation.py:16:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
src/finesse/detectors/amplitude_detector.py:43: in __init__
Detector.__init__(self, name, node, dtype=np.complex128, label="Amplitude", f=f)
src/finesse/detectors/general.py:45: in __init__
ModelElement.__init__(self, name, **kwargs)
src/finesse/element.pyx:103: in finesse.element.ModelElement.__init__
parameter = create_model_parameter(self.__class__, name, value)
src/finesse/element.pyx:50: in finesse.element.create_model_parameter
return type_(name, description, value, unit, flag, cls)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> self.__component_type = component_type
E TypeError: Expected type, got ABCMeta
This was patched by removing all inheritance from ABC, but this is obviously desirable to enforce the use of only public APIs.
Cross-references with the Python API
The user can in the current Python API in master
not do this:
>>> from finesse.components import Laser
>>> x = Laser("l1")
>>> y = Laser("l2", P=x.P)
Exception: Unexpected input '1 W'
If they want to make a reference to x.P
in y
they are supposed to append .ref
to the parameter, like this:
>>> from finesse.components import Laser
>>> x = Laser("l1")
>>> y = Laser("l2", P=x.P.ref)
>>> y.P
<l2.P=l1.P @ 0x7fef76f82a60>
With the changes in this MR, the first example above will currently be allowed, and will set l2
's P
parameter to point to l1
's P
directly rather than via a reference. This is probably not what the user intends, and probably breaks the simulation code somehow since parameters are supposed to be component-specific instances. We should think of some way around this. One idea would be to have a special flag in ModelElement.__init__
which switches off the checks that the passed object is not already a Parameter
, thus allowing the parser to still work:
>>> from finesse.components import Laser
>>> x = Laser("l1")
>>> y = Laser("l2", P=x.P)
Exception: Unexpected input '1 W'
>>> y = Laser("l2", P=x.P, _allow_unref_params=True) # In reality, the flag would only be set by the parser, never elsewhere.
>>> y.P
<l1.P=1 @ 0x7fef76f82550>
It would obviously be silly for the user to use this flag directly, but the parser would be able to use it to get the correct behaviour.
Alternatively we could define a separate init method altogether, which only the parser would use, and the public API would be kept the same (not allowing existing model parameters as arguments).
Or, the parser could set some special field on whatever the element is, e.g. Beamsplitter._dont_check_for_references = True
before calling Beamsplitter.__init__
then resetting it afterwards (context mananger?).
Update from the 2020-11-05 telecon
We decided to have two behaviours depending on the interface. In kat script, specifying R=x.T
would copy x.T
's value to R
and pass the value (float or whatever) to the Python constructor. Using the Python API directly, if the user specified R=x.T
it would throw an error saying the user should specify either .value
or .ref
.
How could this alternatively be achieved?
I suppose another way would be to keep everything as it current is, except allow Parameter
values to be set to other parameters. Then when the Parameter
value setter would see a Parameter
passed, it would overwrite itself with the passed parameter. This seems clunky to me though - creating a Parameter
in __new__
that gets immediately overwritten with whatever the user passes into __init__
, and it would cause derived things like ABCD matrices to get recomputed too. It might also break some weakrefs.
Another way could be to have the parser set some field of the element in question, _use_these_model_parameters
, and __new__
will then use them instead of creating defaults. Again, seems clunky.
Todo
-
Move parameter creation from __new__
to__init__
, and use the passed values instead of defaults if given. -
Change all element constructors to pass arguments to ModelElement.__init__
. -
Get this working with setRTL
inMirror
,Beamsplitter
, etc. and in any other places where model elements are validated before being set in__init__
. -
Add mass
andsignal_gain
model parameters toMirror
? -
Fix ABC issue. -
Allow strings to be set for parameters expecting ints/floats (see #194 (closed)) -
Document the behaviour when calling the constructor in Python with various parameter syntaxes: R=x.T
,R=x.T.value
,R=x.T.ref
,R=float(x.T)
,R=1*x.T
, etc. -
Document the copy by value/reference behaviour when using kat script -
Emit log messages in above cases when parameters get converted to values -
(From telecon) Change component
setter inParameter
to something like_unsafe_set_owner
, since we don't want users calling this setter