Skip to content

Enable use of scheduled hour angle in fba on the fly #175

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sybenzvi
Copy link
Contributor

@sybenzvi sybenzvi commented Jul 2, 2025

PR to address #174. The fba-main-onthefly.sh script can take HA as an argument (calculated externally by NTS) and use it rather than the DESIGN_HA in tiles-main.ecsv.

@sybenzvi sybenzvi marked this pull request as draft July 2, 2025 21:53
@coveralls
Copy link

coveralls commented Jul 2, 2025

Coverage Status

coverage: 30.377% (-0.06%) from 30.435%
when pulling d43852f on use_sched_hourangle
into e4b2ac0 on main.

@sybenzvi sybenzvi requested a review from schlafly July 21, 2025 16:36
@sybenzvi sybenzvi marked this pull request as ready for review July 21, 2025 16:36
@sybenzvi
Copy link
Contributor Author

@schlafly, I could use a sanity check of this PR. The script fba-main-onthefly.sh now handles optional arguments and I altered NTS.py to always pass the current HA of a tile to the script as the scheduled HA. I tried to it with minimal alterations to the existing code.

The change is not backwards compatible with the past ~5 years of fba on the fly because I'm forcing the use of a scheduled HA in the next_tile function. Before we merge this, it's probably a good idea to enable the old behavior by default, with the new behavior trigged by an option passed to next_tile. However, if you think the logic is otherwise OK, that won't be hard to implement.

@schlafly
Copy link
Contributor

That all looks right to me. Too bad that there's not a nicer hour angle interface in desisurvey, but I think what you've done is right. There is no need for compatibility backwards in time with the NTS since we do not do multiple realizations of the survey strategy. I think the logic is good and in your place I would have made it always-on as you have done, without an option for turning it off.

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

Successfully merging this pull request may close these issues.

3 participants