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

Add new features for extension, allowing one program to depend on another #160

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

Conversation

schlafly
Copy link
Contributor

This PR makes a few changes needed to support the DESI extension.

The most important algorithmic change is adding a new config file feature, where a program can be marked as depending on another program, via the following:

programs:
    dark1b:
        depends_on: dark

The intention here is to force the NTS to delay giving out any dark1b tiles until all overlapping dark tiles have been observed.

There are a variety of other minor changes.

  • Extending the stop date for the default ephemerides out to the end of 2029, to accommodate observing farther into the future.
  • new colors for the bright1b and dark1b programs for plotting
  • new arguments to make it easier to change the stop date for optimizing hour angles.
  • updates to surveymovie to enable it to make movies for the extension program
  • deletion of some orphan code I stumbled upon.

Survey simulations with this code are here:
https://data.desi.lbl.gov/desi/users/schlafly/surveysim-20240927/

I think that's all that is needed to do the extension program on the desisurvey/NTS side. I originally intended to also extend the tile file to allow programs to be observed in different types of conditions, but did not actually do that as part of this PR. There is currently a program-to-conditions mapping in the config file, so we won't need to update the tile file to accommodate, but we will need to do some more thinking about how we would want the program selection to look like if we did define a bitmask. We don't need that for the extension and so I've tabled it for now.

@coveralls
Copy link

coveralls commented Sep 30, 2024

Coverage Status

coverage: 30.513% (+0.1%) from 30.403%
when pulling cceb565 on extension
into d97b77b on main.

@schlafly
Copy link
Contributor Author

schlafly commented Jan 7, 2025

@weaverba137 , I think the testing code here is becoming old and having issues. I think to fix the immediate issue we need to add svn as a test dependency, though I don't immediately know how to do that. Do you know?

@weaverba137
Copy link
Member

@schlafly, I didn't quite understand your question, but let me guess: you need svn to download desimodel data, but the GitHub Actions operating system doesn't have it?

@schlafly
Copy link
Contributor Author

schlafly commented Jan 7, 2025

Yes, that's what the tests are now failing to do. It's worked for a long time but presumably something in the github runner config has changed. Note that @dkirkby is the original author here; this PR is unrelated but the tests have started to fail.
https://github.com/desihub/desisurvey/actions/runs/12659961411/job/35280160015?pr=160#step:4:72

@sybenzvi
Copy link

sybenzvi commented Jan 7, 2025

I'm not a workflow expert but since the bot is running ubuntu, can you add

sudo apt-get install subversion

around line 36 of the YAML workflow, just before pip-installing all the python packages? I think the VMs don't require a password for sudo.

@weaverba137
Copy link
Member

@schlafly
Copy link
Contributor Author

schlafly commented Jan 8, 2025

Thanks Ben, Segev. Okay, this passes now. @sybenzvi , if you have any interest in reviewing, please do. There are some changes to the survey movie plots that aren't needed now but make sense to me in general. In principle we could delay those to later but I don't think they're hurting anything now.

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

Successfully merging this pull request may close these issues.

4 participants