Skip to content

Resolve closures before elements are instantiated by parser / remove parser namespaces

finesse importer requested to merge fix/dependency-graph-parsing into master

Fixes #172 (closed).

Summary

  • #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 (model lambda, 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 the Lock constructor 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... Was caused by a bad kat model - thanks Sam.

Details

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.01 and cross references like m m1 R=&m2.R T=0 \n m m2 R=1-&m1.T L=0 are 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_I in this case). Now to ensure that the REFL_I component is parsed ahead of lock in this case, the REFL_I parameter is transformed into a closure by the parser and the builder later transforms this to point to the real REFL_I model 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.value no 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 (closed)).
  • The tracer can no longer rely on the parsed order to determine which cavity to start tracing from. Do we need to add a priority flag somewhere?
  • 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

image

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 Rcx and Rcy parameters).

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...

To-do

  • Get locks working again.
  • Figure out what to do with cavity trace priorities (see #203 (closed))
  • Update #113 (closed) to remove namespace stuff.
  • Try to get self-referencing parameters to work (probably needs changes to Parameter and ModelElement.__new__) - needs lots of work - moved to !39 (closed)
Edited by finesse importer

Merge request reports