desihub / desisurvey

Code for desi survey planning and implementation
BSD 3-Clause "New" or "Revised" License
2 stars 7 forks source link

should desisurvey depend on specsim? #121

Open sbailey opened 4 years ago

sbailey commented 4 years ago

I haven't tracked down why this didn't bite us before, but I just noticed (via desihub/surveysim#73 test failures) that desisurvey.etc depends upon specsim to get the specsim.atmosphere.krisciunas_schaefer model. This breaks our typical rule that pipeline and operations code should not depend upon simulation code.

In particular, specsim isn't on the list of packages that Klaus has installed for ICS, though it appears necessary for ICS to use desisurvey.etc.

I don't see a trivial/obvious solution here since both desisurvey.etc and specsim reasonably would want to have a sky model, and they don't share a common 3rd dependency that we control. A workaround is to accept specsim as a desisurvey dependency, but I think the rule that "ops/pipeline code shouldn't depend upon simulation code" is well motivated so I don't want to give it up lightly.

@dkirkby @schlafly @daqiii or others, thoughts?

sbailey commented 4 years ago

Correction: apparently specsim is installed at KPNO; I was being fooled by a mismatch of an old system-level "pydoc" at KPNO vs. the python that is actually used, which is different issue. So this ticket isn't a blocking factor for our current ICS installation.

But I still question whether operations code should depend upon simulation code. Discuss.

dkirkby commented 4 years ago

I agree that specsim should not be a dependency.

I have some work in progress to move the sky model into a new pkg that both specsim and desisurvey use, and propose that we leave the specsim dependency in as a placeholder until that is ready.