Resolve closures before elements are instantiated by parser / remove parser namespaces
Fixes #172 (closed).
- #172 (closed) is fixed by implementing topological sorting and building of a proper abstract syntax tree during parsing.
- By necessity, all references between instructions are resolved before build, so #130 (closed) is again broken, but I think we should fix that in a different way.
- I took the liberty to remove namespace support in kat script (
analysis xaxis, etc.). In hindsight this just added complexity for little gain. Now adapters can have flags set to determine if they have a name and can be specified multiple times which should cover all of our current intended instructions.
- Do we need a way to determine cavity trace priority now?
Something broke with locks; I'm getting singular matrices and I can't tell why. One thing that changed is that theWas caused by a bad kat model - thanks Sam.
Lockconstructor now gets the parameter object of its target rather than a string, but as far as I can tell from the code this should be getting correctly handled...
The solution decided in #172 (closed) is not trivial to implement: to resolve closures before building the model, the model has to be built in the correct order, with components that depend on (contain references to) other components being instantiated first.
Since part of the solution in #172 (closed) is to remove the possibility to make self and cyclic references between elements, the kat script abstract syntax tree (AST) can now be represented as a directed acyclic graph. This has the property of at least one topological order that can be followed to allow the model elements to be built in a way that avoids unresolved dependencies. This MR introduces this ordering.
The use of an AST and topological ordering allows a slight reduction in the complexity of the current build step, since different types of closure are now no longer treated differently (previously analyses, model parameters and model parameter references were all treated slightly differently). However, this also makes it possible to specify functions and references as non-model parameters in init calls, which #130 (closed) originally fixed; however, I think if we want to fix this again we should do so by sanitising these values in init methods of each element rather than in the parser, since this is also a problem for the Python API.
Some slight changes in behaviour:
- Self references like
m m1 R=1-&m1.T T=0.01and cross references like
m m1 R=&m2.R T=0 \n m m2 R=1-&m1.T L=0are no longer allowed.
Lock.__init__was previously given an element name as a string when parsing e.g.
lock lock_length REFL_I ETM.phi -1 1e-5(
REFL_Iin this case). Now to ensure that the
REFL_Icomponent is parsed ahead of
lockin this case, the
REFL_Iparameter is transformed into a closure by the parser and the builder later transforms this to point to the real
REFL_Imodel element (having been built first).
Lock.__init__was previously set up to receive a string as input, not an object, so this needed fixed.
- Specifying something like
var order 2 \n modulator mod1 10M 0.1 1+&order.valueno longer throws an error in the parser (or in this case, at all). Here a non-model parameter, modulation order, is being set with a reference to a variable's value. This is currently possible via the Python API too, so we should probably rather add the error checking into
Modulator.__init__etc. (see #86).
- The tracer can no longer rely on the parsed order to determine which cavity to start tracing from. Do we need to add a
- The AST is now explicitly built by the parser.
The AST is stored in
KatYACC.ast and can be viewed with e.g.
networkx.draw or (better) graphviz via
networkx.drawing.nx_pydot.write_dot(parser.ast, "ast.dot") then
dot -T pdf -O ast.dot.
Here is an example script and its AST:
l L0 P=1 s s0 L0.p1 EOM1.p1 mod EOM1 ( f=100M midx=0.1 order=1 mod_type=pm ) s s1 EOM1.p2 ITM.p1 m ITM R=1-&ETM.T T=&ETM.T s sCAV ITM.p2 ETM.p1 L=1 m ETM R=1 T=0 phi=0.1 pd1 REFL_I ITM.p2.o &EOM1.f 0 lock lock_length REFL_I ETM.phi -1 1e-15 run_locks lock_length
A topological order in this case would be:
['ETM', 'ITM', 'sCAV', 'EOM1', 'REFL_I', 'lock_length', 'run_locks', 's1', 'L0', 's0']
(Another valid order would be top to bottom in the graphviz example above.)
Why self-references aren't allowed
As stated above, instructions with parameters that reference other parameters in the same statement ("self-reference" parameters) are not allowed any more. These can in principle be stated in a way that's perfectly valid (such as
m m1 R=1-&m1.T T=0.01, in contrast to e.g.
m m1 R=1-&m1.T T=1-&m1.R which would be impossible), but because the parser doesn't build
Parameter objects itself and instead leaves this up to the relevant object constructor, it can't pass a resolved parameter self-reference to the constructor (simply, the
Parameter can't be referenced if it doesn't exist yet).
It would take a lot of effort to allow this to work. We would first of all have to build
Parameter objects in the parser rather than the constructor, which would break the principle of the constructor being the only place these parameters get made, which might lead to subtle bugs down the line if we do anything smart during parameter creation (for instance, I haven't thought whether this would work with the transparent mapping of single
Rc values in the
Surface constructor to separate
Secondly, the parser would have to build an AST not just with instructions as nodes (as shown in the graph above), but with parameters as nodes too, with edges connecting parameters to their owning instructions. Parameters referencing other parameters would be connected by edges, and as long as the graph had a topological order it could be built in a way that everything could resolve (including parameters referencing others in the same element). And because parameters would have to in some cases get instantiated before their owning element, we would have to modify
Parameter to not have an
owner, or at least not rely on having an owner when it first gets made and instead have it set in a later step.
There are probably other challenges, such as how to give a globally unique identifier to references to parameters defined by position in a statement (I think this is solvable though, I had to solve that for the generator already), but it should be discussed whether this is worth it supposedly just be able to do
m m1 R=1-&m1.T T=0.01 rather than
m m1 T=0.01 L=0...