lsst-sims / legacy_sims_movingObjects

3 stars 5 forks source link

Feature/exact ephs #26

Closed rhiannonlynne closed 6 years ago

rhiannonlynne commented 6 years ago

This PR significantly updates the generation of observations. It adds the capability to generate observations at the exact time of each observation, rather than just using a linear interpolation over a grid of times. It also adds the option to use a rectangular footprint, as well as the circular and camera footprints. makeLSSTobs.py has added more options to take advantage of these new capabilities. The final output observations now contain metadata regarding their generation. Updated examples notebooks.

yoachim commented 6 years ago

One issue, my eups didn't have pandas and it's not an intuitive thing to trick eups into installing it (turns out you need to specify a version). Maybe easiest solution is to just add setupRequired(pandas) to the lsst_sims table? Then it should get taken care of like numpy, etc.

Unit tests are failing all over the place (same way on my local install and if I try it in docker).

The example notebooks run great! Maybe put instructions to sym-link the opsim sqlite into the example directory, then you can remove your hard-coded path.

rhiannonlynne commented 6 years ago

I'm not sure I understand about "your eups doesn't have pandas" -- are you using the LSST miniconda? The LSST miniconda comes with pandas now.

yoachim commented 6 years ago

yup, I have pandas with miniconda. But if pandas is in the ups table file there needs to be a stub package for eups to "setup". Just like there's a numpy one even though numpy comes with conda.

rhiannonlynne commented 6 years ago

But pandas is in the table file already?

On Wed, Oct 3, 2018 at 11:15 AM Peter Yoachim notifications@github.com wrote:

yup, I have pandas with miniconda. But if pandas is in the ups table file there needs to be a stub package for eups to "setup". Just like there's a numpy one even though numpy comes with conda.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lsst/sims_movingObjects/pull/26#issuecomment-426741242, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxQ39_TgKq_vRnw2n6DwozoXTu5oCfgks5uhP60gaJpZM4XEiZD .

rhiannonlynne commented 6 years ago

(it's in the table file for sims_movingObjects)

Sorry - maybe I'm not understanding.

Pandas is in the table file for sims_movingObjects. There is also a stub package for pandas. If you have it in your miniconda, I think you should be fine? How did you install and setup sims_movingObjects?

yoachim commented 6 years ago

The problem is if one does eups distrib install lsst_sims -t sims clone and declare sims_movingObjects setup sims_movingObjects will fail because eups has not installed a pandas package.

rhiannonlynne commented 6 years ago

Yes. sims_movingObjects is not part of lsst_sims and would not be installed (or any of its dependencies) by installing lsst_sims. This was done on purpose because of complaints about the fortran compiler required for oorb. You also cannot "eups distrib install sims_movingObjects" yet becuase we haven't published a build.

We could add pandas to lsst_sims as a dependency, but it is not actually needed by any of the lsst_sims packages. But it's pretty handy, so maybe worthwhile.

However, you can use lsstsw to "rebuild -r feature/exact_ephs sims_movingObjects" and get it and all the dependencies. (of course, the unit test failures you pointed out mean that sims_movingObjects won't finish its install, but the code would be there to test, in lsstsw/build/sims_movingObjects .. you can setup the package even while it's there by noting the build number, cd lsstsw/build/sims_movingObjects, and doing "setup -r . -r bXXXX').

Presumably at this point, with the solar system science collaboration starting ot use sims_movingObjects, it would be worthwhile to do a 'publish' step once this is finished, and then an "eups distrib install sims_movingObjects" would work (if the user had fortran available).

yoachim commented 6 years ago

Seems like it's going to be hard to get anyone else to use it if you require lsstsw to install. I guess if you're planning on publishing a build, then that solves the problem.

rhiannonlynne commented 6 years ago

It's hard to get anyone to use it even if you just "eups distrib install" following "newinstall" (which we can do, publishing a build is fine ... it's just not been published yet or if it has, it's a very old build).

TBH, from what I've seen, lsstsw is currently in better working shape than newinstall. They go back and forth though.

But yeah - publishing a docker build is good, and access on jupyterhub is better (see https://epyc.astro.washington.edu/sssc/hub/login for the SSSC)

rhiannonlynne commented 6 years ago

For what you want to do right now - get pandas acknowledged -- I think you should be able to do "eups distrib install pandas" maybe? Another option is to just (since you have pandas installed in your system): eups declare pandas system -r none -m none

yoachim commented 6 years ago

turns out one needs to do eups distrib install pandas -v master-g4426870190.

rhiannonlynne commented 6 years ago

The unit test failures were rather stupid problems. Fixed.

rhiannonlynne commented 6 years ago

https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/28764/pipeline in progress

okay, after some more changes to loosen the tolerance on the ephemeris generation (compared to JPL .. still within absolute tolerances, but had to change the recorded values), it does now actually build.

Merge?

rhiannonlynne commented 6 years ago

Merged and deleted feature/exact_ephs