search

McStasMcXtrace / mccode-antlr

McCode grammar implemented with ANTLR4
1 stars 0 forks source link

`Instr.split` does not check for broken component instance references in the second instrument #32

Closed g5t closed 10 months ago

g5t commented 11 months ago

When using Instr.split a flag can be set to check for and remove instrument parameters that are no longer needed in either of the two resulting instrument. https://github.com/McStasMcXtrace/mccode-antlr/blob/c2e1e69017b8f051493832f22117cebe4d57f820/mccode_antlr/instr/instr.py#L359

There are no checks, however, to verify that no dependency exists across the split point for component instance positions or orientations. Since we can fully resolve these dependency chains we should at least be able to identify when a split instrument will fail to compile due to use of undefined variables. More usefully, if the check is performed before removing unused parameters, it should be possible to correct the problematic reference(s) automatically.

g5t commented 11 months ago

The implementation in #33 is insufficient, and likely requires changing the overall behavior of Instr.split to better support Instr.mcpl_split.

At present an instrument with a components tuple like (instance_a, instance_b, ..., instance_m, instance_n, instance_o, ..., instance_y, instance_z); calling split with instance_n would produce two Instr objects with components tuples (instance_a, instance_b, ..., instance_m, instance_n), and (instance_o, ..., instance_y, instance_z), respectively.

Instead, if the split-point component instance was present in both Instr objects (or perhaps only in the second one), then relative positioning will remain simpler in the second Instr object.

g5t commented 10 months ago

Instead, if the split-point component instance was present in both Instr objects (or perhaps only in the second one), then relative positioning will remain simpler in the second Instr object.

This was done in https://github.com/McStasMcXtrace/mccode-antlr/commit/ab306e2346c3b4e041ac3b65765800c9d1922ec9 For lack of better recollection, this issue likely should have been closed at that point and I am closing it now.