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

set version to 0.5.0-rc1 #3196

Conversation

psychocoderHPC
Copy link
Member

@psychocoderHPC psychocoderHPC commented Mar 3, 2020

ATTENTION: the rc1 release branch is the destination not dev

@psychocoderHPC psychocoderHPC added the documentation regarding documentation or wiki discussions label Mar 3, 2020
@psychocoderHPC psychocoderHPC added this to the 0.5.0 milestone Mar 3, 2020
Copy link
Member

@PrometheusPi PrometheusPi left a comment

Choose a reason for hiding this comment

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

The ax3l repo is still used in various places. Should this stay this way? (If not, I will approve this pull request.)

@@ -1,10 +1,10 @@
Bootstrap: docker
From: ax3l/picongpu:0.5.0
From: ax3l/picongpu:0.5.0-rc1


%labels
Maintainer Axel Huebl <a.huebl@hzdr.de>
Copy link
Member

Choose a reason for hiding this comment

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

@psychocoderHPC Should @ax3l stay the maintainer or should it shift to you?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this to the todo list: https://github.com/ComputationalRadiationPhysics/picongpu/projects/25
I changed the version with our version changing tool, I would keep it "wrong" because I assume @ax3l 's docker image points to the dev branch. @ax3l is this correct?

Copy link
Member Author

@psychocoderHPC psychocoderHPC Mar 5, 2020

Choose a reason for hiding this comment

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

Let us wait with merging for a reply of @ax3l to know how he handled it in the past.

Copy link
Member

Choose a reason for hiding this comment

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

Usually the container images and :tag that I upload correspond to the release, while :latest then also points to the last release.

We do not distribute the singularity container on our own, as you can see this is just a plain old import from the Dockerfile. But Nvidia distributes a singularity container that way via NGC, which they build on their own with this.

Don't forget to shoot an e-mail to @AdamSimpson (please CC me on it) when the release is done and a new docker image is pushed to dockerhub, so he can update it on NGC (as well as the singularity container build from it). We usually do this after the release, so in a couple of weeks from now.

@@ -1,10 +1,10 @@
Bootstrap: docker
From: ax3l/picongpu:0.5.0
Copy link
Member

Choose a reason for hiding this comment

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

This should be pointing to anther repo not ax3l

@@ -56,20 +56,20 @@ You can also push the result to dockerhub and singularity-hub (you need an accou
cd ubuntu-1604

# docker image
docker build -t ax3l/picongpu:0.5.0 .
docker build -t ax3l/picongpu:0.5.0-rc1 .
Copy link
Member

Choose a reason for hiding this comment

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

ax3l repo su still used. Is this correct?

@@ -25,7 +25,7 @@ This exposes the ISAAC port to connect via the webclient to.
.. code:: bash

docker pull ax3l/picongpu
docker run --runtime=nvidia -p 2459:2459 -t ax3l/picongpu:0.5.0 lwfa_live
docker run --runtime=nvidia -p 2459:2459 -t ax3l/picongpu:0.5.0-rc1 lwfa_live
Copy link
Member

Choose a reason for hiding this comment

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

ax3l repo is still used

Copy link
Member

Choose a reason for hiding this comment

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

Please tell me if you (e.g. @PrometheusPi and @psychocoderHPC ) create a dockerhub account.
I already reserved a picongpu/ org a while back and will then add you to it :)
https://hub.docker.com/orgs/picongpu

Copy link
Member

Choose a reason for hiding this comment

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

"Register now and increase the bus-factor by a three-fold."™

Copy link
Member Author

Choose a reason for hiding this comment

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

@ax3l I am now registered at docker.com as psychocoderhpc

@@ -6,7 +6,7 @@ ENV DEBIAN_FRONTEND=noninteractive \
FORCE_UNSAFE_CONFIGURE=1 \
SPACK_ROOT=/usr/local \
SPACK_EXTRA_REPO=/usr/local/share/spack-repo \
PIC_PACKAGE='picongpu@0.5.0+isaac backend=cuda'
PIC_PACKAGE='picongpu@0.5.0-rc1+isaac backend=cuda'
Copy link
Member

@ax3l ax3l Mar 5, 2020

Choose a reason for hiding this comment

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

This tag here is the tag you provide in our spack repo.

So add this in its packages/picongpu/package.py before you can build.

@@ -6,7 +6,7 @@ ENV DEBIAN_FRONTEND=noninteractive \
FORCE_UNSAFE_CONFIGURE=1 \
Copy link
Member

Choose a reason for hiding this comment

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

Above: the MAINTAINER labels seams to be deprecated now in Docker. Just use:

LABEL maintainer="Rene Widera <r.widera@hzdr.de>, Axel Huebl <a.huebl@hzdr.de>"

@@ -1,10 +1,10 @@
Bootstrap: docker
From: ax3l/picongpu:0.5.0
From: ax3l/picongpu:0.5.0-rc1


%labels
Maintainer Axel Huebl <a.huebl@hzdr.de>
Copy link
Member

Choose a reason for hiding this comment

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

This seams to work here, too:

Suggested change
Maintainer Axel Huebl <a.huebl@hzdr.de>
Maintainer "Rene Widera <r.widera@hzdr.de>, Axel Huebl <a.huebl@hzdr.de>"

Copy link
Member Author

Choose a reason for hiding this comment

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

I hoped that it is not coming to the point that I need to maintain the docker too^^
I will work on all the docker stuff end of next week.

@PrometheusPi
Copy link
Member

@psychocoderHPC what is the status of this pull request?

@psychocoderHPC
Copy link
Member Author

Last time I got spack issue during OpenMPI was compiled. I have currently no time to look into it.

@psychocoderHPC
Copy link
Member Author

currently my docker tests ended with:

[ 95%] Building NVCC (Device) object CMakeFiles/picongpu.dir/picongpu_generated_main.cpp.o
nvcc fatal   : Unknown option '-diag_suppress'
CMake Error at picongpu_generated_versionFormat.cpp.o.Release.cmake:220 (message):
  Error generating
  /opt/picInputs/lwfa/.build/CMakeFiles/picongpu.dir//./picongpu_generated_versionFormat.cpp.o


CMakeFiles/picongpu.dir/build.make:87: recipe for target 'CMakeFiles/picongpu.dir/picongpu_generated_versionFormat.cpp.o' failed
make[2]: *** [CMakeFiles/picongpu.dir/picongpu_generated_versionFormat.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
nvcc fatal   : Unknown option '-diag_suppress'
CMake Error at picongpu_generated_main.cpp.o.Release.cmake:220 (message):
  Error generating
  /opt/picInputs/lwfa/.build/CMakeFiles/picongpu.dir//./picongpu_generated_main.cpp.o

@psychocoderHPC
Copy link
Member Author

currently my docker tests ended with:

[ 95%] Building NVCC (Device) object CMakeFiles/picongpu.dir/picongpu_generated_main.cpp.o
nvcc fatal   : Unknown option '-diag_suppress'
CMake Error at picongpu_generated_versionFormat.cpp.o.Release.cmake:220 (message):
  Error generating
  /opt/picInputs/lwfa/.build/CMakeFiles/picongpu.dir//./picongpu_generated_versionFormat.cpp.o


CMakeFiles/picongpu.dir/build.make:87: recipe for target 'CMakeFiles/picongpu.dir/picongpu_generated_versionFormat.cpp.o' failed
make[2]: *** [CMakeFiles/picongpu.dir/picongpu_generated_versionFormat.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
nvcc fatal   : Unknown option '-diag_suppress'
CMake Error at picongpu_generated_main.cpp.o.Release.cmake:220 (message):
  Error generating
  /opt/picInputs/lwfa/.build/CMakeFiles/picongpu.dir//./picongpu_generated_main.cpp.o

OK I think I found the issue. We have a broken alpaka CMake file. I will fix it directly in the subtree (third party folder). This will simplify later the merge from master into dev (less conflicts compared to a subtree update with a fixed alpaka).

@psychocoderHPC
Copy link
Member Author

We need to release ISAAC before this release, else the spack recipe will not work.

@psychocoderHPC
Copy link
Member Author

I found the problem, I found a workaround but this workaround broke windows compile with separate compile enabled.
But the real problem why -Xcudafe was removed when it was multiple times used as option is an issue in ISAAC. ISAAC removes duplicated options in CUDA_NVCC_FLAGS see here

@psychocoderHPC
Copy link
Member Author

For documentation: a fix of alpaka CMake ref would solve the issue. Never the less I will remove the critical option in ISAAC instead of changing the alpaka cmake. The reason is that ISAAC is not adding any option to CUDA_NVCC_FLAGS thus it should not manipulate the cmake variable.

@psychocoderHPC psychocoderHPC force-pushed the topic-versionUpdateTo0.5.0-rc1 branch from e9d3a17 to d7e1317 Compare May 4, 2020 14:48
@sbastrakov
Copy link
Member

Is it now ready for another review?

@psychocoderHPC
Copy link
Member Author

Yes can be merged now. ISAAC 1.5.0 is released which fixes all issues. I will update the spack recipes after we released PIConGPU

Copy link
Member

@sbastrakov sbastrakov left a comment

Choose a reason for hiding this comment

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

Not pressing merge right now, maybe @PrometheusPi would like to have another look.

@psychocoderHPC
Copy link
Member Author

@sbastrakov you can merge it, there will be another PR too to change the version to 0.5.0 later.

@sbastrakov sbastrakov merged commit b3e34c6 into ComputationalRadiationPhysics:release-0.5.0-rc1 May 12, 2020
@psychocoderHPC psychocoderHPC deleted the topic-versionUpdateTo0.5.0-rc1 branch May 13, 2020 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation regarding documentation or wiki discussions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants