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

should desisurvey depend on specsim? #121

Open
sbailey opened this issue Nov 4, 2020 · 2 comments
Open

should desisurvey depend on specsim? #121

sbailey opened this issue Nov 4, 2020 · 2 comments
Assignees

Comments

@sbailey
Copy link
Contributor

sbailey commented Nov 4, 2020

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
Copy link
Contributor Author

sbailey commented Nov 4, 2020

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
Copy link
Member

dkirkby commented Nov 4, 2020

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.

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

No branches or pull requests

4 participants