Cache specsim

[sbailey]

This PR makes the quickbrick and quickgen tests ~2x faster by caching the specsim.simulator.Simulator object instead of recreating it anew every time. It also enables these tests on Travis using specsim master and desisim master, now that specsim fits within the memory limits of Travis (as a side effect, the tests were also faster due to that change, even before this caching). After the next tag of specsim, we can update this to use that tag instead.

Since the cached Simulator object may have been altered by the previous test, the caching code also caches the state for a few key variables and puts them back to their original state before returning the cached object:

• qsim.source.focal_xy
• qsim.atmosphere.airmass
• qsim.observation.exposure_time
• qsim.atmosphere.moon.moon_phase
• qsim.atmosphere.moon.separation_angle
• qsim.atmosphere.moon.moon_zenith

Are there other variables that should also be reset?

The new desisim.specsim module could also be the namespace landing spot for other desi-specific wrappers to specsim to unify the common code from quickbrick and quickgen. If having both desisim.specsim and specsim is confusing, please suggest an alternate name for desi-specific convenience wrappers of specsim.

[dkirkby]

I think we ideally want a working (and unit tested) deepcopy for Simulator objects, so desisim doesn't have to know the details of what needs to be copied and won't break when that changes.

Does the fact that travis tests are passing on this PR mean that your list above is sufficient for these tests?

[sbailey]

I think we ideally want a working (and unit tested) deepcopy for Simulator objects, so desisim doesn't have to know the details of what needs to be copied and won't break when that changes.

Agreed. This is a workaround in the meantime to make it easier to work with these tests.

Does the fact that travis tests are passing on this PR mean that your list above is sufficient for these tests?

Yes. I pulled the list of items to cache and reset from the list of items that quickbrick / quickgen update (in their code and in their tests).

Under normal use, calling quickbrick or quickgen will only create one Simulator object anyway so this issue doesn't apply. If one is concerned that the reset isn't being done completely, one can always call specsim.simulator.Simulator(config) as before and be guaranteed a fresh copy ~9 seconds later.

[dkirkby]

With the test config, the Simulator ctor only takes ~0.5s, so I suspect that most of the extra desi-config setup time is spent reading and parsing large ascii files. Just replacing these with binary files could make a big dent in those ~9 seconds.

[sbailey]

Checking on the status of this. Some alternate suggestions have been raised:

• update desimodel to use binary files that are faster to read than the ascii files
• update specsim.simulator.Simulator to work with deepcopy (see Support deepcopy of Simulator objects specsim#51; the underlying issue is that astropy.coordinates.AltAz doesn't support deepcopy)

Either of these is admittedly better, but they need more work and in the meantime this PR has a pragmatic fix. Any objections to merging this now and then replacing it with a better solution if/when those are ready?

[dkirkby]

The current solution is a bit fragile (since it copies a list of attributes by hand that depends on the exact tests being run), but I am ok with merging now and then updating after desihub/specsim#51 is resolved.

[sbailey]

I agree that it is fragile. If we continue to use this as-is and some additional parameter gets added we'll probably forget to update it here and then I'll have to do some penance. But noting:

• it doesn't impact normal usage of quickgen and quickbrick since those only create a single Simulator. It is only an issue for tests or other cases that want to create multiple Simulators.
• I'm happy to replace it with a deepcopy of a pristine cached Simulator once that works (Support deepcopy of Simulator objects specsim#51)

Merging now as a pragmatic step toward better testing...