Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates needed to run on-site #86

Open
kmharrington opened this issue Oct 19, 2023 · 6 comments
Open

Updates needed to run on-site #86

kmharrington opened this issue Oct 19, 2023 · 6 comments

Comments

@kmharrington
Copy link
Member

kmharrington commented Oct 19, 2023

  • ability to set scan parameters before scans start (Expand Options for Scan definition #71)
  • acu.generate_scan accepts an az_speed and az_accel those need to be configurable (and the defaults should be 1 deg/s and 1 deg/s**2.)
  • seq.scan already assumes we're at a starting point while acu.generate_scan is happy to move us to the starting point. It'd be good to be able to look at the middle of the scan for detector setup and then move to the edge, especially when the edge is close to the sun
  • smurf.stream needs to accept subtype as well as tag and seq.scan needs to pass those along.
@mhasself
Copy link
Member

  • acu.generate_scan accepts an az_speed and az_accel those need to be configurable (and the defaults should be 1 deg/s and 1 deg/s**2.)

To be clear -- do you mean seq.scan? Assuming you do, are you saying the default values (1, 1) should live in the seq.scan implementation? If so, we will probably need them to be a config parameter, settable per-platform?

The ACU agent now has a notion of "default" scan params; those can be tweaked through set_scan_params. So nextline (or whatever) scripts could use that, at the top, to set the defaults. (Then, the defaults are a scheduler config thing, instead of a sorunlib config thing.) And it is easy to change the scan params, for a bunch of scans in a row, by adding one line to a script.

@kmharrington
Copy link
Member Author

I guess meant that since acu.generate_scan accepts az_speed and az_accel I'd like seq.scan to accept it as well. Right now they're hard coded to 2 deg/s and 2 deg/s^2 there. We haven't verified those speeds yet with the cryostat so I can't call this function right now.

Yes to configurable in a config file in the future but right now I'd like like to be able to call the function at all.

@BrianJKoopman
Copy link
Member

The ACU agent now has a notion of "default" scan params; those can be tweaked through set_scan_params. So nextline (or whatever) scripts could use that, at the top, to set the defaults. (Then, the defaults are a scheduler config thing, instead of a sorunlib config thing.) And it is easy to change the scan params, for a bunch of scans in a row, by adding one line to a script.

What's the ACU's default behavior, say if you haven't called set_scan_params() yet, but then don't pass az_speed and az_accel to generate_scan()?

@mhasself
Copy link
Member

What's the ACU's default behavior, say if you haven't called set_scan_params() yet, but then don't pass az_speed and az_accel to generate_scan()?

Answered offline :) Agent default defaults, on startup, are here.

@BrianJKoopman
Copy link
Member

  • ability to set scan parameters before scans start (Expand Options for Scan definition #71)

  • acu.generate_scan accepts an az_speed and az_accel those need to be configurable (and the defaults should be 1 deg/s and 1 deg/s**2.)

Addressed in #88.

  • smurf.stream needs to accept subtype as well as tag and seq.scan needs to pass those along.

Addressed in #87.

  • seq.scan already assumes we're at a starting point while acu.generate_scan is happy to move us to the starting point. It'd be good to be able to look at the middle of the scan for detector setup and then move to the edge, especially when the edge is close to the sun

This one requires some coordination with the scheduler. I'd still like the final script syntax to end up looking like:

acu.move_to(az, el)
smurf.bias_dets()
wait_until('2023-07-07T08:27:00+00:00')
smurf.bias_step()
seq.scan(description='description text', stop_time='2023-07-07T09:12:00+00:00', width=30)
smurf.bias_step()

Or something similar. seq.scan() starts at one edge, and goes +width in azimuth before turning around for a scan. When @mhasself and I talked about this early on this was what we preferred.

Do we shift to just starting the scan in the middle then? This would mean instead we'd have:

generate_scan.start(az_endpoint1=az - width/2, az_endpoint2=az + width/2, ...)

And the first part of the scan would be moving towards the start point, then actually scanning.

Alternatively the scripts move to a slightly more verbose:

acu.move_to(az, el)  # center of field
smurf.bias_dets()  # or whatever else you need to do to prep detectors
acu.move_to(az2, el2)  # edge of field
wait_until('2023-07-07T08:27:00+00:00')
smurf.bias_step()
seq.scan(description='description text', stop_time='2023-07-07T09:12:00+00:00', width=30)
smurf.bias_step()

And seq.scan() remains unchanged. That puts this more firmly on the scheduler's end.

@msilvafe
Copy link

Or something similar. seq.scan() starts at one edge, and goes +width in azimuth before turning around for a scan. When @mhasself and I talked about this early on this was what we preferred.

Do we shift to just starting the scan in the middle then? This would mean instead we'd have:

generate_scan.start(az_endpoint1=az - width/2, az_endpoint2=az + width/2, ...)

I think az - width/2 to az + width/2 would make the convention consistent with the scan parameters returned in the obsdb obsinfo. Keeping those consistent I think will prevent confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants