Skip to content
This repository has been archived by the owner on Dec 9, 2022. It is now read-only.

packaged ptychography software (frontend + backend) #486

Merged
merged 12 commits into from
Jun 7, 2019

Conversation

leofang
Copy link
Collaborator

@leofang leofang commented Jan 3, 2019

I finally packaged the ptychography code deployed at HXN and CSX. There're things to be discussed offline with @licode and perhaps @stuartcampbell before merging. Among those, the usage of git submodule, currently hosted in my private repo in order to avoid patent issues, may be an obstacle for the building system? Perhaps moving both repos (GUI and GPU code) to NSLS-II and allowing read-only access for the building agent is the best option? (We need to grant people from other nat'l labs read-only access anyway.)

@mrakitin
Copy link
Member

mrakitin commented Jan 3, 2019

Thanks @leofang.

Did you try to build it locally with conda build . in the recipe folder? You can use a Docker image from https://cloud.docker.com/u/nsls2/repository/docker/nsls2/debian-with-miniconda or https://cloud.docker.com/u/mrakitin/repository/docker/mrakitin/debian-with-miniconda to try it - they have preinstalled conda and necessary packages.

Also, is https://github.com/leofang/ptycho_gui the private repo you are talking about? I think I can access it. You probably meant your personal repo. I checked the structure of the project briefly, and it seems it won't install as the setup.py is missing. You can consider https://github.com/NSLS-II/scientific-python-cookiecutter to prepare a proper structure of the project to make it pip-installable.

@leofang
Copy link
Collaborator Author

leofang commented Jan 3, 2019

Did you try to build it locally with conda build . in the recipe folder?

Yes, I did conda build --python 36 nsls2ptycho in lightsource2-recipes/recipes-tag/ and then conda install --use-local nsls2ptycho. After that I tested the installed package, and it worked as expected.

You can use a Docker image from https://cloud.docker.com/u/nsls2/repository/docker/nsls2/debian-with-miniconda or https://cloud.docker.com/u/mrakitin/repository/docker/mrakitin/debian-with-miniconda to try it - they have preinstalled conda and necessary packages.

Sorry I don't understand the reason for using a Docker image, @mrakitin could you please explain?

Also, is https://github.com/leofang/ptycho_gui the private repo you are talking about? I think I can access it. You probably meant your personal repo. I checked the structure of the project briefly, and it seems it won't install as the setup.py is missing. You can consider https://github.com/NSLS-II/scientific-python-cookiecutter to prepare a proper structure of the project to make it pip-installable.

The project has two parts: a frontend (a GUI) which is https://github.com/leofang/ptycho_gui as you noticed, and a backend (CPU+GPU codes) which is hosted in private mode https://github.com/leofang/ptycho --- I don't think you could access this! The backend is set to be a git submodule of the frontend, see the ptycho folder in https://github.com/leofang/ptycho_gui/tree/packaging/nsls2ptycho/core.

Ah, I see the confusion. The packaging branch in the ptycho_gui repo is pip-installable, which is what I specified in the recipe. I'll merge it to master later. When you clone the frontend repo, in the source you can do pip install . I made sure of this before moving to write a conda recipe based on pip install. Does this make sense to you?

Thanks!

@tacaswell
Copy link
Member

Sorry I don't understand the reason for using a Docker image, @mrakitin could you please explain?

so that we can control the underlying enviroment that the package is built against. One of the more annoying things that can happen is you build a package on your (up-to-date) laptop and then it will not run on the machines on the floor because the binaries are linked against a newer version of libc than is available on the floor.

@mrakitin
Copy link
Member

mrakitin commented Jan 3, 2019

@leofang, thanks for elaborating on the private repo, now I understand. Yes, as @tacaswell explained, we want a reproducible environment, which is provided by the docker images mentioned above. Just run a container as:

$ docker pull nsls2/debian-with-miniconda
$ docker run -it --rm nsls2/debian-with-miniconda bash

and you will have a fresh environment for conda build. Clone the repo, switch to your branch, and built the package as you did.

@leofang
Copy link
Collaborator Author

leofang commented Jan 3, 2019

Great, thanks @mrakitin @tacaswell. I built the package on xf03id-srv5 since they have GPUs that I can test right after building and installing, but I'll try using docker and report here later.

@leofang
Copy link
Collaborator Author

leofang commented Jan 15, 2019

I've followed @mrakitin's instruction to build with docker and it worked after a few slight changes in meta.yaml and setup.py (mainly to address "numpy header not found"). I also added two import tests as suggested by @licode.

The only thing left before making all these work is to mirror the backend to the internal GitLab server so that the builder can access it, but I don't know how to do this.

@mrakitin mrakitin added this to the Undefined milestone Mar 15, 2019
@leofang
Copy link
Collaborator Author

leofang commented Mar 21, 2019

@mrakitin the backend has been moved to internal GitLab, and a local build was successful. However, during building I still need to type my GitLab id and pw because the repo's visibility is set to "internal". Any idea to get around this without making the repo public?

@mrakitin
Copy link
Member

@leofang, do you have a GitLab token? Maybe we can use it to authenticate and clone the code from your private repo?

@leofang
Copy link
Collaborator Author

leofang commented Mar 23, 2019

I saw that page too. But I think our internal GitLab is not on the latest version, so it lacks many features including deploy tokens. This is what I see in Settings->Repository.

@leofang
Copy link
Collaborator Author

leofang commented Mar 24, 2019

I took some time to investigate, and my understanding is

  1. Both GitHub and GitLab support repo-level deploy keys (ssh keys) and user-level tokens (called Personal Access Token).
  2. With a user-level token, creating a git submodule using a remote url like https://leofang:[email protected]/leofang/ptycho.git will make git clone work with my private repo.
  3. However, the token gives full access to everything under the user's account, and since at the submodule level the TOKEN is stored as plain text, we don't really wanna expose it as it's unsafe.
  4. So we wanna use deploy keys. I tested that if we create a ssh key pair in the docker image, copy the public key to my private repo as a deploy key, and in the git submodule use a url https://[email protected]/leofang/ptycho.git (no need to use TOKEN this time), git clone will also work.

So, my new question is: is it unsafe to store the ssh key pair from ssh-keygen in a docker image? If it's ok, then we can just update the docker image and everything else will work.

@mrakitin
Copy link
Member

mrakitin commented Mar 24, 2019

Great job on investigating the options! It is save to store .pub part of the ssh key pair, but never the private pair. What we do for other tokens is passing them via env variables to the Docker container when we start it. That way we don't have to worry about storing them in the Docker image. If you want to use the ssh pair, we can mount it if the secret dir exists on our building server (e.g., /home/youruser/secrets/ --> /secrets/).

@leofang
Copy link
Collaborator Author

leofang commented Mar 25, 2019

If you want to use the ssh pair, we can mount it if the secret dir exists on our buildind server (e.g., /home/youruser/secrets/ --> /secrets/).

Could you clarify this? What you want to mount? What is to be put in the secrets dir?

@mrakitin
Copy link
Member

If you place your public/secret keys somewhere on freyja, we can mount docker image with the -v option (see docs here). We already mount the cloned repo here:

-v $REPO_ROOT:/repo \

We can do something similar with your keys.

As this package is very special, for now I think we will be fine with accepting a built package from you, which can be put to our local channel on alexandia manually without external mirroring. You could provide a ready package (tar.bz2), I can place it to the local channel.

specifying attrs version is mainly for resolving the dep correctly
@leofang
Copy link
Collaborator Author

leofang commented Mar 27, 2019

@mrakitin I gave up ssh for a number of reasons that we can discuss offline. Now I switch back to personal access tokens in f8878ad. The key insight is that https://[email protected]/leofang/ptycho_gui.git is a valid git url (no user name is needed! FYI, this doesn't work for GitLab.) and TOKEN will propagate all the way to the submodules, therefore it's not needed to expose it as plain text anywhere.

I tested in docker and it works, so we're only one step away: Is it possible to set an environmental variable named GH_TOKEN before the build starts? (I store my token on Freyja so it should be accessible there.)

As this package is very special, for now I think we will be fine with accepting a built package from you, which can be put to our local channel on alexandia manually without external mirroring. You could provide a ready package (tar.bz2), I can place it to the local channel.

OK. This will be the last resort.

Let me know what you think. Thanks!

@mrakitin
Copy link
Member

Great progress, @leofang! The problem now is that freyja will push every built package to our public channel; we don't have the machinery not to push (yet). I think it will need some preparation before we can proceed. On the package side, we may need some flag to mark the package as "private".

@leofang
Copy link
Collaborator Author

leofang commented Mar 27, 2019

Ah, that's right. We discussed this long ago on Slack. Glad we're finally here! I suppose this could be a good summer project? 😆

So for the time being let's use the last resort. I think when you manually place the package to the local channel, it won't be uploaded, right?

@mrakitin
Copy link
Member

Ah, that's right. We discussed this long ago on Slack. Glad we're finally here! I suppose this could be a good summer project? 😆

Our building approach will be significantly reworked. I guess freyja will only be needed for such corner cases.

So for the time being let's use the last resort. I think when you manually place the package to the local channel, it won't be uploaded, right?

Correct, the mirroring only happens in one direction: https://anaconda.org/lightsource2-tag/ --> alexandria.

@mrakitin
Copy link
Member

Here is a summary about gitlab ssh access: https://3.basecamp.com/3733838/buckets/4534819/messages/1693665280.

@leofang
Copy link
Collaborator Author

leofang commented May 11, 2019

Just leaving a note for myself when I return to this PR in the near future.

Resolving the CUDA dependency issue for Conda packages is still challenging. One of the reasons is that currently cudatoolkit is an incomplete package that only contains runtime libraries and lacks the compiler nvcc and friends. While it is impossible at the Conda level to install the CUDA driver (which requires root), it should be possible for Conda to infer the driver version and install the toolkit with the highest version supported by the driver. Indeed, this is an ongoing effort that will be included in the latest Conda release, see discussions in conda-forge/conda-forge.github.io/issues/687 and conda/conda/pull/8267. Hopefully, we will be able to add something like - cudatoolkit >=9.0 in the build section of the recipe. (And as users, conda install cudatoolkit=9.0) Then, we can work toward packaging ptychography, tomography, and more software that offer GPU support more easily.

For this particular package, in this branch I've resolved the GPU & CuPy issues at the pip level by assuming GPU may not exist. If detected, nvcc will be invoked to generate CUDA binaries that will be pip installed, and the corresponding version of CuPy will be added as a dependency (if not already installed). So, once the new cudatoolkit recipe is ready, at the Conda level we will be able to provide a nvcc (regardless of whether it's already available or not) by adding cudatoolkit to the build section as mentioned above so that the pip solution will always have a nvcc to work with.

Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we put the recipe into a separate folder, such as recipes-manual or recipes-private. This way they won't be attempted to build on freyja, but we still could have a recipe to build manually. Same may work for https://github.com/NSLS-II/playbooks/issues/278.

@leofang
Copy link
Collaborator Author

leofang commented Jun 6, 2019

recipes-private sounds better as it indicates the recipes in this folder won't (and can't) go public. If it's OK to you I'll proceed.

@mrakitin
Copy link
Member

mrakitin commented Jun 6, 2019

Yes please, let's do it. This way we may have subsequent PRs to that folder rather than keeping iterating in this single WIP PR.

@leofang leofang changed the title WIP: packaged ptychography software (frontend + backend) packaged ptychography software (frontend + backend) Jun 6, 2019
@leofang
Copy link
Collaborator Author

leofang commented Jun 6, 2019

Done. Question: if this PR is merged now, would it trigger the builder (I hope not)?

@mrakitin
Copy link
Member

mrakitin commented Jun 6, 2019

Thanks Leo. It won't trigger a build as it only tracks the changes in recipes-tag:

last_updated="$(git log --pretty=format: --name-only --since="${how_long}" | grep recipes-tag/ | cut -d/ -f2 | sort -uR)"

and the building itself is happening here:
./repo/scripts/build.py /repo/recipes-tag/${pkg_name} -u $UPLOAD_CHANNEL --python 3.6 3.7 --numpy 1.14 --token $BINSTAR_TOKEN --slack-channel $SLACK_CHANNEL --slack-token $SLACK_TOKEN --allow-failures

Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general. Left a couple of comments to simplify the structure a bit and about the private repo.

recipes-private/nsls2ptycho/build.sh Outdated Show resolved Hide resolved
recipes-private/nsls2ptycho/build.sh Show resolved Hide resolved
recipes-private/nsls2ptycho/meta.yaml Show resolved Hide resolved
recipes-private/nsls2ptycho/meta.yaml Show resolved Hide resolved
recipes-private/nsls2ptycho/meta.yaml Outdated Show resolved Hide resolved
@mrakitin mrakitin merged commit 53e57e2 into NSLS-II:master Jun 7, 2019
@leofang
Copy link
Collaborator Author

leofang commented Jun 7, 2019

🍺

@mrakitin
Copy link
Member

mrakitin commented Jun 7, 2019

🍻

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants