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

Migrate to single status file; rudimentary afternoon planning; allow tile & status files to not match. #108

Merged
merged 27 commits into from
Jun 25, 2020

Conversation

schlafly
Copy link
Contributor

This PR and an associated one on surveysim reorganizes the desisurvey plan & scheduler state files into a single state file. It also adds rudimentary afternoon planning code which scans for DESI spectra for ETC information and fiberassign directories for available tiles, but I will need to revisit the details of these bits once NERSC is back up. Finally, it allows the derived available exposure & status files to no longer exactly match the tile file---i.e., if we have some weird commissioning TILEIDs, or if we change tile files, etc., that should not cause any issues.

There are also a few miscellaneous changes, like letting NTS be passed an "azrange" argument, which limits selected tiles to a particular azimuth range.

This PR does not make any changes to the actual survey simulations; the same tiles get scheduled at the same times.

The associated PR on surveysim updates surveysim to use the slightly changed API in this patch, primarily to desisurvey.plan.Plan, which now is instantiated with a simulate=True argument if simulated fiber assignment is desired, and which now uses the single state file when --save-restore is requested.

There's more to do here. Some points for discussion:

  • I'm now imagining that each night's desisurvey configuration is a config.yaml file and a status file. The config file also specifies the rules.yaml file to be used for that night. I can imagine the config.yaml file changing over the course of the survey, and so checking these in and making them a part of the history of the planning of each night seems sensible to me. On the other hand, a lot of the information in config.yaml is not particularly variable.
  • NTS currently only avoids rescheduling via QueuedList and makes no attempt to keep state up to date otherwise (i.e., tile completion and donefrac statistics). The assumption is that these will get updated during the next afternoon planning sequence.
  • The nightly planning files are then currently:
    1. config.yaml
    2. rules.yaml
    3. queued-YYMMDD.csv
    4. desi-status-YYMMDD.csv
  • These files are currently dropped in DESISURVEY_OUTPUT, though I'll push everything into a DESISURVEY_OUTPUT/YYMMDD directory soon.
  • I'm imagining the state files get produced by afternoon planning and then are never touched each night. They can safely be checked in and forgotten about. Afternoon planning will then need to gain some smarts about checking out and parsing other human files overriding nominal completeness statistics from the ETC/offline pipeline/...
  • Currently, all DESI files get scanned for ETC headers each night. We should consider less redundant alternatives.

The equivalent PR on surveysim currently fails testing because it depends on the new API. I'm not sure how to tell CI that it needs to test relative to a special branch.

@schlafly schlafly requested a review from dkirkby February 25, 2020 00:19
Copy link
Member

@dkirkby dkirkby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schlafly Do you want to merge this now, or continue working in this PR?

For the corresponding surveysim tests, you could modify the travis rules to pip-install a particular branch from github, or just wait until this is merged to master.


def add(self, tileid):
self.queued.append(tileid)
open(self.fn, 'a').write(str(tileid)+'\n')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there is an IO error during this file open & write?

@dkirkby
Copy link
Member

dkirkby commented Feb 25, 2020

I am a bit concerned about the proliferation of blocks wrapped with if self.simulate: since, ideally, this package should behave identically in real vs simulated ops. There probably indicates that some refactoring between desisurvey and surveysim is needed, which could be in a future PR.

@schlafly
Copy link
Contributor Author

Let me continue working on this PR to put a few more updates in so that afternoon planning runs on ~real fiberassign and desi raw spectra files. I'll try to get that in this weekend and will hope to merge early next week.

@schlafly
Copy link
Contributor Author

I added a recent set of commits here that supports afternoon planning like so:
python afternoon_plan.py --night 20200611 --config config-cmx.yaml --restore_etc_stats etc_stats_20200610.fits
This produces in $DESISURVEY_OUTPUT the directory 20200611 with the following contents:

CMX_58_DITHERED_TILES_Jan2020_s.fits  # the tile file used
config-cmx.yaml  # the config file used, edited to point to this directory
desi-status-20200611.fits  # the status file used
etc_stats-20200611.fits  # the etc_stats file used
rules-cmx.yaml  # the rules file used

These are intended to be all of the files necessary to reproduced afternoon planning results at a later time. We expect that things like the tile file are near-constant, so it may be silly to have a copy of these for each night, but it seems reasonable to expect changes at some point in the survey and copying it into the afternoon planning directory seemed reasonable. Likewise the config.yaml file and rules.yaml file are expected to be unchanged. The etc_status file gives the ETC completion statistics on each tile based on scanning the desi/data/spectro dir, and the desi-status file tracks things like which tiles are available, priorities, etc..

One ugly feature of this model is that one runs afternoon planning pointing to one config / rules / tile file, and then afternoon planning creates new copies of these in the nightly afternoon planning directory. The new config file is then edited to point to the new copies of the rules and tile file. I have some slightly ugly code identifying which lines of the yaml file are the relevant ones to edit, which will be a bit fragile; we could imagine different schemes.

Then the NTS is started pointing to this directory. It should not need any files outside of this directory.

>>> nts = NTS.NTS('20200611/config-cmx.yaml')
>>> nts.next_tile(59012.3, program='CMX_DITHERING')                        
{'tileid': 63138,
 's2n': 50.0,
 'esttime': 12307.54078161631,
 'maxtime': 1200.0,
 'fiber_assign': '/global/cfs/cdirs/desi/target/fiberassign/tiles/trunk/063/fiberassign-063138.fits',
 'foundtile': True}

I have chosen to eliminate the fiber_assign_dir argument to the NTS initializer; this information is now in the config.yaml file. It is needed there anyway so that we can scan to see which fiberassign files are available for observation. I am now defining the "obsplan" argument as pointing to the appropriate config.yaml file for this set of observations, which in turn points to the appropriate rules and tile file in the afternoon planning directory.

I think the above works. Some rough edges we should discuss:

  • this code requires in a couple places the ability to load desisurvey.config.Configuration(), and then load it again with its new, updated config file. I know that David Kirkby is not a fan of this. The calls to Configuration.reset() are ugly. I also need to clear out the tile file and load it back in to clear the cache when it changes. But I think we want this ability in any case, since I don't want to require an ~ICS reset if these things change, and NTS/desisurvey is operating as a module ICS is instantiating.
  • I mentioned that I'm not happy with the way I take the config.yaml file passed to afternoon planning on the command line, copy it to the new afternoon planning directory, and edit it to point to only things also copied into that directory. That feels fragile.

@dkirkby
Copy link
Member

dkirkby commented Jun 12, 2020

Is it still possible to run a surveysim of the nominal 5-year survey after these changes? How about one night of CMX or SV?

I am not too worried about using Configuration.reset() for this very specific use: it is part of the public API after all.

@schlafly
Copy link
Contributor Author

You can run a 5 year surveysim after merging the corresponding surveysim PR. Reviewing that now, though, it looks like we need to update the rules.yaml file to use the new tile file, for which gray is pass 0. A current 5 year run with the default rules file finishes way too late.

I'll start trying to understand the rules file syntax, but the default rules.yaml file is rather more complicated than the rules-depth.yaml file, so if you're game to update it, please do.

I haven't tried to simulate a night of CMX or SV. Maybe that's useful. I think if I use the usual fiberassign / completeness framework that should be easy, but might not test very much.

@schlafly
Copy link
Contributor Author

Yes, after fixing the rules file I get what looks like a pretty familiar surveysim run, though I haven't looked at these in a while... the intent was certainly to induce no changes on the simulations.
image

@schlafly
Copy link
Contributor Author

I'm merging this. More can be done, which I've described in the following issues:

However, what's in there should be in a good place for integrating with ICS, which is a higher priority at the moment.

@schlafly schlafly merged commit 29aa164 into master Jun 25, 2020
@schlafly schlafly deleted the statusfile branch September 8, 2020 22:39
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.

2 participants