Dealing with tests that require desimodel

weaverba137 commented 8 years ago

Creating an issue to track a discussion I just had with @sbailey.

Some tests in the desisim test suite require desimodel, both code and data. This is a serious problem for Travis tests because:

desimodel is not public.
Even if desimodel were public, it would be a large download to install it for the purposes of a Travis test.
LBNL security might start worrying about random cloud instances downloading 100s of MB, dozens of times (assuming we expand the test matrix).
Even assuming a desimodel install is successful, how do we ensure that desisim can find the data?

Some solutions that have been discussed so far:

Make desimodel public.
Create a custom Docker instance that already contains desimodel, and test using that. This assumes Travis allows custom Docker instances.
Separate the desimodel code from the data, and make the code public.

And an additional idea:

Make smaller versions of the files and incorporate these directly into the package structure of desimodel or desisim.

weaverba137 commented 8 years ago

Anyone know what the difference between Docker and Container is?

By default Travis uses Containers to run tests. I have also confirmed that they can run Docker.

weaverba137 commented 8 years ago

Pasting a reply that Peter sent to me directly (please respond on GitHub not by email!):

Docker is a lightweight linux container. I've not heard of containers before as a product. NERSC is moving towards something they call Shifters, which is a spin-off of Docker.

Thanx. I didn't mean to imply that Travis is using a product called "containers", it's just that they use the term 'container-based infrastructure' a lot without directly referencing Docker. In other words, I'm trying to figure out if 'container-based infrastructure' is the same as 'Docker' or if they mean something different.

nugent68 commented 8 years ago

Ahh, there are several container-based infrastructures. Docker is just the most popular.

Cheers,

Peter

On Wed, Nov 4, 2015 at 10:13 AM, Benjamin Alan Weaver < notifications@github.com> wrote:

Pasting a reply that Peter sent to me directly (please respond on GitHub not by email!):

Docker is a lightweight linux container. I've not heard of containers before as a product. NERSC is moving towards something they call Shifters, which is a spin-off of Docker.

Thanx. I didn't mean to imply that Travis is using a product called "containers", it's just that they use the term 'container-based infrastructure' a lot without directly referencing Docker. In other words, I'm trying to figure out if 'container-based infrastructure' is the same as 'Docker' or if they mean something different.

— Reply to this email directly or view it on GitHub https://github.com/desihub/desisim/issues/37#issuecomment-153814274.

Peter E. Nugent Division Deputy for Scientific Engagement Computational Research Division, LBNL Adjunct Professor of Astronomy, UC Berkeley M.S. 50B-4206 - 1 Cyclotron Road - Berkeley, CA, 94720-8139 Phone:(510) 486-6942 - Fax:(510) 486-4300 - Cell:(925) 451-4001 E-mail: penugent@LBL.gov - Web: http://www.lbl.gov/~nugent http://www.lbl.gov/%7Enugent

sbailey commented 8 years ago

We have the green light from Michael to make desimodel public. Let's open that up and experiment with having the Travis tests checkout desimodel trunk on-the-fly and see if that is viable.

weaverba137 commented 8 years ago

I've sent a request to Jeff Anderson, who'll have to tweak the configuration for this. In the meantime, could you please review the desimodel branch that implements the pip-install stuff: https://desi.lbl.gov/svn/code/desimodel/branches/pip-install/

sbailey commented 8 years ago

That branch looks reasonable, but I'm not particularly qualified to catch errors on pip-install stuff.

weaverba137 commented 8 years ago

My remaining concerns about desimodel are:

pip install will not necessarily install the data directory.
There is really very, very little Python code left; did I strip out too much?

sbailey commented 8 years ago

It is absolutely crucial that the data directory is installed — that is the main point of desimodel; the remaining I/O code is just in support of that data and is useless without it. If that fundamentally doesn't work with pip, then ok, we may just need different instructions. It could be as simple as:

svn checkout https://desi.lbl.gov/code/desimodel/trunk desimodel
export DESIMODEL=pwd/desimodel
export PYTHONPATH=$DESIMODEL/py/:$PYTHONPATH

Or perhaps the last point is handled via a pip install of the code portion, while still needing to keep an svn checkout and setting $DESIMODEL by hand.

weaverba137 commented 8 years ago

I am already working on code to do this. Basically there are two use cases:

At NERSC, desiInstall will handle installing the data directory, without any need for additional work.
For personal installs, the pip install can provide a script that will install the data directory.

weaverba137 commented 8 years ago

I'm making progress on the desimodel install, however, I just discovered that desisim depends on desimodel.focalplane, which I had assumed was no longer needed because only desimodel.io was needed. What do we do about this?

sbailey commented 8 years ago

I had forgotten about the dependency upon FocalPlane. That could get moved into desispec,

sbailey commented 8 years ago

clicked wrong button too soon ... re-opening ... FocalPlane could get moved into desispec, though that isn't a completely natural location for it either. desisim is only using it for entering placeholder values into ra,dec,x,y in the fibermap, and those are currently not used. Fiber assignment needs similar xy <-> RA,dec mapping code, but it currently does that in C++ land.

Note: desisim/quickgen also still expects to find quicksim in desimodel, which has also been factored out in this branch. We intend to update quickgen to use specsim instead, but that hasn't happened yet. So it is good to clean up desimodel, but we aren't ready for the trimmed down version quite yet.

weaverba137 commented 8 years ago

Here's a suggested procedure for installing desimodel on personal systems:

desimodel_tag=1.0.0
pip install svn+https://desi.lbl.gov/svn/code/desimodel/tags/${desimodel_tag}#egg=desimodel
export DESIMODEL=${HOME}/desimodel/${desimodel_tag}
mkdir -p ${DESIMODEL}
svn export https://desi.lbl.gov/svn/code/desimodel/tags/${desimodel_tag}/data ${DESIMODEL}/data

This seems simple enough to me that a script isn't necessary, we just add these instructions to the README file.

dkirkby commented 8 years ago

@sbailey Specsim also needs various focal plane transforms and is not supposed to depend on any DESI code, so I have already implemented them in specsim.transform. The ra,dec --> x,y is implemented in two separate steps, to isolate the atmospheric refraction physics from the purely geometric transformations. First, convert ra,dec to alt,az, then convert alt,az to x,y. In the reverse direction, I have implemented x,y --> alt,az but not alt,az -> ra,dec. Do you need that also?

sbailey commented 8 years ago

desisim.targets.get_targets() needs an x,y -> ra, dec transformation (going via alt,az is fine). If you could implement the final alt,az -> ra,dec step in specsim that would be helpful. In desispec.targets.get_targets(), this is just for getting a distribution of objects that are realistically associated with fibers and with locations on the sky for organizing into bricks.

If we find that desispec or other non-simulation code needs these transforms (e.g. a python refactor of fiberassign) then we should replicate or pull that piece out into one of the non-simulation packages.

weaverba137 commented 8 years ago

Could I get someone to re-review the "pip-install" branch of desimodel? There is still some work to do on a script that will install the data directory in non-NERSC systems.

dkirkby commented 8 years ago

Is it ok if I make a PR for the pip-install branch, to more easily see and comment on the changes? Is there a pip command I can use to actually test installing this branch?

weaverba137 commented 8 years ago

Go ahead.

Both the desisim and the datamodel branches should be installable:

pip install git+https://github.com/desihub/desisim@pip-install#egg=desisim
pip install svn+https://desi.lbl.gov/svn/code/desimodel/branches/pip-install#egg=desimodel

weaverba137 commented 8 years ago

This is partially, but not completely fixed by #55. We had to skip a bunch of tests that require desimodel data as opposed to desimodel code.

sbailey commented 8 years ago

desisim now makes use of the lightweight test-0.4 branch of desimodel data, allowing much more complete test coverage. Closing issue.

desihub / desisim

Dealing with tests that require desimodel #37