search

nasa / dorado-scheduling

Dorado observation planning and scheduling simulations
Other
21 stars 8 forks source link

Unify main.py and survey.py #42

Closed lpsinger closed 3 years ago

lpsinger commented 3 years ago

It seems like there are very few important differences between these two files. The only differences that I can see are minor variations in the command line arguments.

mcoughlin commented 3 years ago

How do you feel about a class representing the survey? Right now, you calculate the times and such in main.py, which is currently factored out into the Survey class. https://github.com/nasa/dorado-scheduling/blob/main/dorado/scheduling/models.py This is the main difference currently. I think once this is settled. We can get these merged.

lpsinger commented 3 years ago

I am OK with exposure time being part of the Mission class. I want the time step to be part of the command line arguments, though. Same for the HEALPix resolution. Also I now am drawing a distinction between the HEALPix grid that is used for spatial calculations and the tessellation grid that specifies the allowed pointings of the telescope.

mcoughlin commented 3 years ago 
p.add_argument('--skygrid-step', type=u.Quantity, default='0.0011 sr',
               help='Sky grid resolution (any solid angle units')
p.add_argument('--skygrid-method', default='healpix',
               choices=[key.replace('_', '-') for key in skygrid.__all__],
               help='Sky grid method')

Maybe you also want to support one of the tiling files as an optional option? This would help bring it into alignment.

lpsinger commented 3 years ago

Why do we need to support providing the list of tiles as a file? Generating the tile grid is nearly instantaneous.

mcoughlin commented 3 years ago

But presumably we want a tile file like ZTF's (and UVEX) for the facilities that will pin the grid? Even if the grid is reproducible, people will want the file..

lpsinger commented 3 years ago

This is a good point. We can use argparse's add_mutually_exclusive_group to select between those two alternatives. Would you like to do the honors of preparing a PR to merge these two scripts?

mcoughlin commented 3 years ago

Fixed by #44