Allow floating point values

[prkrtg]

No description provided.

[astronomerdave]

I made a change to wavegen.py which allows constants to print as floats. All constants will now be floats. If that is not acceptable then we'll need another solution.

[astronomerdave]

This is what you were missing.

[weatherhead99]

any reason this can't be an f-string?

[astronomerdave]

All constants will now be floats. If that is not acceptable then we'll need another solution.

[weatherhead99]

constants are allowed to be integers by the archon, but I don't really know why you'd use it that way so I don't see a problem here.

We also need an implementation allowing a constant to appear as either a voltage or a slew rate in any SET statement, and allowing same to appear as the named slew rate in driver module definitions.

[astronomerdave]

@weatherhead99: We have I think the capability that you want. You can define "constants" in the .def file such as
#define Foo 1.1
and then use
SET S1 TO Foo
in your .waveform file. See the demo files, for example. This is not using Archon Constants but I think it does effectively the same thing -- it allows you to have "constants" which can be used anywhere in your waveforms by name.

[weatherhead99]

Hi @astronomerdave I'm well aware we can do that. But it doesn't provide the functionality I want, that's the whole reason for the issue.

Say I have a constant defined at the preprocessor level. In order to programmatically change the value of that, I need to (from my program), sed the WDL source code, shell out to the WDL compiler, reload the whole acf into archon. This is acceptable for e.g. slew rates. But not for e.g. parallel clock levels.

If I have them defined as a "constant" within the archon itself, for excample say I have CONSTANT1=LLEL_HIGH=9.0 or similar. And in the timing script somewhere I have a state which references "LLEL_HIGH". I then just need to overwrite a single archon config line, and do an "APPLYTIMING" to have the value of that voltage adjusted. Which is a trivial operation from a driver program.

[weatherhead99]

let's say for example I want to run a test where I take 3 images at each of 30 different parallel clock settings...

[weatherhead99]

p.s. I believe I'm using all the functionality available (and quite a bit more e.g. using GPP's #exec directive to shell out to an externnal calculator to calculate the values for slew rates.... if I'm missing something here please do point it out, trake a look at https://github.com/CaltechOpticalObservatories/wdlfiles/blob/deimos_coincident/src/deimos/deimos.seq and see what you think, it can likely be done a lot more elegantly

[astronomerdave]

I'm well aware we can do that. But it doesn't provide the functionality I want, that's the whole

OK, I understand now.

[astronomerdave]

@prkrtg was this supposed to be working? I can define a const but not use it.

[weatherhead99]

seems to almost but not quite work. Here is the test case I tried first:

const c1 = 12.0

WAVEFORM testwf {
  SET [1:2] to c1;
 
}

SEQUENCE testseq {
   testwf();
}

gives the following error:

  File "/home/danw/Software/wdl/venv/bin/wdl", line 8, in <module>
    sys.exit(main())
             ~~~~^^
  File "/home/danw/Software/wdl/wdl/cli.py", line 53, in main
    return obj(cli_mode=True)
  File "/home/danw/Software/wdl/wdl/commands/legacy_drivers.py", line 92, in __call__
    output: str = Parser.parse(self._text)
                  ~~~~~~~~~~~~^^^^^^^^^^^^
  File "/home/danw/Software/wdl/wdl/wdlParser.py", line 2644, in parse
    waveform_text += waveform()
                     ~~~~~~~~^^
  File "/home/danw/Software/wdl/wdl/wdlParser.py", line 2068, in waveform
    waverules()
    ~~~~~~~~~^^
  File "/home/danw/Software/wdl/wdl/wdlParser.py", line 2028, in waverules
    to()
    ~~^^
  File "/home/danw/Software/wdl/wdl/wdlParser.py", line 1947, in to
    error("Expected level as number or defined constant after TO")
    ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/danw/Software/wdl/wdl/wdlParser.py", line 186, in error
    token.abort(msg)
    ~~~~~~~~~~~^^^^^
  File "/home/danw/Software/wdl/wdl/genericToken.py", line 119, in abort
    raise LexerError(
    ...<10 lines>...
    )
wdl.genericToken.LexerError: 
In line 4 near column 16:

  SET [1:2] to c1;
               ^

Expected level as number or defined constant after TO

so I added a semicolon, and tried this test case:

const c1 = 12.0

WAVEFORM testwf {
  SET [1:2] to c1;
 
}

SEQUENCE testseq {
   testwf();
}

which gives:

Traceback (most recent call last):
  File "/home/danw/Software/wdl/venv/bin/wdl", line 8, in <module>
    sys.exit(main())
             ~~~~^^
  File "/home/danw/Software/wdl/wdl/cli.py", line 53, in main
    return obj(cli_mode=True)
  File "/home/danw/Software/wdl/wdl/commands/legacy_drivers.py", line 92, in __call__
    output: str = Parser.parse(self._text)
                  ~~~~~~~~~~~~^^^^^^^^^^^^
  File "/home/danw/Software/wdl/wdl/wdlParser.py", line 2662, in parse
    error("(wdlParser.py::parse) unrecognized token " + token.show(align=False))
    ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/danw/Software/wdl/wdl/wdlParser.py", line 186, in error
    token.abort(msg)
    ~~~~~~~~~~~^^^^^
  File "/home/danw/Software/wdl/wdl/genericToken.py", line 119, in abort
    raise LexerError(
    ...<10 lines>...
    )
wdl.genericToken.LexerError: 
In line 1 near column 16:

const c1 = 12.0;
               ^

(wdlParser.py::parse) unrecognized token Symbol:;

note that I ran this through the CLI tool I made in my branch, rather than directly into wdlParser.py I can't see why that would cause this issue but if you think it does my apologies.

[weatherhead99]

here is a test case which I believe shows all relevant uses of constants that we care about

const c1 = 12.0
const c2 = 550.0

WAVEFORM testwf {
  SET [1:2] to c1, FAST;
 
}

SEQUENCE testseq {
   testwf();
}

SLOT 1 driverx
{
  DRVX 1 [c2, 1.0, 1] "clock1";
}

[weatherhead99]

so this does appear now to allow me to define constants and use them in the places I expect. However, it breaks previously used module definitions, in particular the XVBIAS definition. The following code was valid on my branch:

SLOT 4 xvbias {
  PBIAS 1 0 [0,0];
  PBIAS 2 0 [0,0];
  PBIAS 3 0 [0,0];
  PBIAS 4 0 [0,0];  
  NBIAS 1 1 [0, -0] "SCI Backside";
  NBIAS 2 0 [0,-0];
  NBIAS 3 0 [0,-0];
  NBIAS 4 0 [0,-0];
}

but now breaks with a parsing error on this PR branch

[weatherhead99]

(BTW I actually have no idea why the syntax is like that maybe @astronomerdave can shed some light. It would make as much sense to me if it was spelled as

PBIAS 1 [0, 0, 0];
to bring it into line with DriverX modules. But I also don't know how much/ many WDL code there is out there that relies on this behaviour

[astronomerdave]

(BTW I actually have no idea why the syntax is like that maybe @astronomerdave can shed some light.

Other than "that's the way it is", I don't have any explanation as to why PBIAS and NBIAS have their enable/disable flag outside of the square brackets. I wrote this almost 10 years ago and I don't remember any special reason, and it could be that no one used those commands before and/or just never gave feedback.

[weatherhead99]

I did check by the way and it gives me the same parsing error if I write e.g.

NBIAS 1 [1, 0, -0] "SCI Backside";

so that is not actually the issue with this particular situation

[weatherhead99]

this now appears to work for everything I need in the short term. Maybe we should document the fact that const definitions cannot appear in a .mod file, and only in .seq file (and .waveform file???)

[weatherhead99]

all LGTM, tested on my branch building current DEIMOS waveform definitions and all appears working as intended. Suggest merging this into main as it shouldn't break anything existing that wasn't already broken.

[weatherhead99]

any reason this can't be an f-string?