ProjectQ-Framework / FermiLib

FermiLib: Open source software for analyzing fermionic quantum simulation algorithms
https://projectq.ch/
Apache License 2.0
86 stars 38 forks source link

Linked Docker image in README. #74

Closed hsim13372 closed 7 years ago

hsim13372 commented 7 years ago

I created a separate repo (link in the README) that has a Dockerfile and instructions to build and run a Docker image that contains FermiLib and ProjectQ.

damiansteiger commented 7 years ago

Thanks for your suggestion to add a Docker image. We are currently testing the performance impact of using Docker compared to the new simplified installation method of ProjectQ (added yesterday to the develop branch and later this week to pypi). We are currently also discussing where to add a potential Dockerfile (our main ProjectQ repo, both repos, or maybe even better just on our website www.projectq.ch).

Regarding this pull request: Links to repo's outside of the ProjectQ-Framework or our official website are not allowed.

babbush commented 7 years ago

Indeed, we need to discuss how the Docker image a bit more. But why are links to outside repos not allowed?

damiansteiger commented 7 years ago

In this case user experience and quality reasons.

We would love to keep the external dependencies as low as possible. In this case, it would be weird if a user has to go to an external repository in order to install FermiLib. Also we want to keep full control of all the installation methods and all core parts of FermiLib such that we can provide fast fixes if something goes wrong.

The default to adding external links is no, as we most likely can fix it with an internal solution. In this case, after evaluating the Docker solution, we could provide a Dockerfile on our site with installation instructions. That said, there are cases where we might allow links to external sites/repos but we will carefully review them but please bring them up if you think it solves an important problem.

jarrodmcc commented 7 years ago

I'm personally of the opinion that maintaining a Docker file on something like Dockerhub has no downsides. It offers a great option for trying things out and even running them on non-standard hardware like supercomputers, arm architectures, AWS, Microsoft's Azure etc. where docker is maintained for this reason.

The issue of where to keep such an image is separate. I understand your arguments related to having it externally maintained, however with a sufficient number of caveats in a Readme I don't think having "Third-party plugins" or some such section is a bad thing. We need to strike a balance between maintaining the aesthetic integrity of the repo and locking out additions that show people use and maintain the library who aren't just us.

That said, I'd be all for an official "Project-Q" DockerHub repo.

hsim13372 commented 7 years ago

@damiansteiger, those are good points. The external repo isn't actually necessary as long as the user has the Dockerfile (essentially just a text file) or can pull from the Docker Hub. I created a separate repo just so that I can have the instructions there instead of crowding the READMEs of FermiLib and ProjectQ. Also, it's very easy to set up an automatically building Docker Hub repo if you already have a GitHub repo containing the Dockerfile.

babbush commented 7 years ago

Hi Hannah, thanks for making these contributions. However, would you mind breaking up unrelated changes into distinct pull requests? Generally speaking we should try to keep each pull request limited to a specific set of related changes. This helps make the reviewing task easier and is a generally good engineering practice. For instance, it looks like many of your commits are uncontroversial improvements which I would be happy to approve and merge into FermiLib right now. But I can't do that without accepting the docker PR which is something we're still discussing (although perhaps we should have a meeting about this next week with @damiansteiger and @jarrodmcc)?

In order to make different pull requests you should create different branches (one per pull request) on your private fork. I see that you are making all of these changes on the develop branch of your private fork but I would not recommend doing that. Suppose you wanted to make a pull request to add the docker image. I suggest that you "git branch docker_image" (docker_image is now the name of the feature branch) and then "git checkout docker_image" in order to switch to that feature branch. Then submit a pull request about the docker image (for instance). This also allows you to keep your develop branch synced with the official develop branch without messing it up. If you need to edit something about your docker_image pull request then you can do so on the docker_image branch but otherwise you should start a totally new branch if you are adding or changing unrelated parts of the code. Once a pull request is accepted you can delete the feature branch (e.g. "git branch -d docker_image") and pull the changes into your forked develop branch from the official development branch.

At this point it might be somewhat complicated to separate your commits. Something you might consider doing is to copy all of your new and changed files somewhere on your local machine and then delete your fork. Then refork a clean version of the repo and add the new and changed files one at a time back to the new fork but on appropriate feature branches (not on the develop branch of your fork) from which you make the pull requests. There are more elegant ways to do this but I'm explaining here the approach I would probably take in your shoes at this point. Let me know if this all makes sense.

hsim13372 commented 7 years ago

Hi Ryan,

Sorry about that! I'm still new to using git and didn't realize I was doing this! I'll take your suggestions and fix these soon.

Best, Hannah

Sent from my iPhone

On Jun 30, 2017, at 3:51 PM, Ryan Babbush notifications@github.com wrote:

Hi Hannah, thanks for making these contributions. However, would you mind breaking up unrelated changes into distinct pull requests? Generally speaking we should try to keep each pull request contained to specific changes. This helps make the reviewing task easier and is a generally good engineering practice. For instance, it looks like many of your commits are uncontroversial improvements which I would be happy to approve and merge into FermiLib right now. But I can't do that without accepting the docker PR which is something we're still discussing (although perhaps we should have a meeting about this next week with @damiansteiger and @jarrodmcc)?

In order to make different pull requests you should create different branches (one per pull request) on your private fork. I see that you are making all of these changes on the develop branch of your private fork but I would not recommend doing that. Suppose you wanted to make a pull request to add the docker image. I suggest that you "git branch docker_image" (docker_image is now the name of the feature branch) and then "git checkout docker_image" in order to switch to that feature branch. Then submit a pull request about the docker image (for instance). This also allows you to keep your develop branch synced with the official develop branch without messing it up. If you need to edit something about your docker_image pull request then you can do so on the docker_image branch but otherwise you should start a totally new branch if you are adding or changing unrelated parts of the code. Once a pull request is accepted you can delete the feature branch and pull the changes into your develop branch.

At this point it might be somewhat complicated to separate your commits. Something you might consider doing is to copy all of your new and changed files somewhere on your local machine and then delete your fork. Then refork the repo and add the new and changed files one at a time back to the new fork but on appropriate feature branches (not on the develop branch of your fork) from which you make the pull requests. There are more elegant ways to do this but I'm explaining here the approach I would probably take in your shoes at this point. Let me know if this all makes sense.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

babbush commented 7 years ago

Closing after speaking with Hannah. She will reopen separate pull requests with same content.