-
Notifications
You must be signed in to change notification settings - Fork 7
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
base: main
Are you sure you want to change the base?
Conversation
…o depend on another program.
@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? |
@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? |
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. |
I'm not a workflow expert but since the bot is running ubuntu, can you add
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. |
That's also what I recommend. You can see a working example here: https://github.com/desihub/gpu_specter/blob/27cdd1bcc032e586adc75c002b5ef7bdb554b97f/.github/workflows/python-package.yml#L30 |
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. |
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:
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.
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.