Closed bernt-matthias closed 2 years ago
I would have done some of the work, but @bernt-matthias has this branch on his fork.
As a maintainer you are allowed to push to the branch of my fork, but I'm happy to fix it myself :).
Therefore, here are the things we need to address:
* The branch we were working on for the revision has been merged to master. We need to check this commit to ensure everything is fine. * We have updated the required package versions and need to merge the `environment.yml` on master
Yes.
* Most importantly we made substantial changes to the source code during the revision. We also deleted all references to the _standalone_ Keras and use the one that is part of TensorFlow. Essentially, we need to throw away Matthias' changes of the `dfpl` code and bring them up to date.
This is easy.
* We need to verify that the examples and scripts we provide still work.
So add tests here and to the PR workflow here https://github.com/yigbt/deepFPlearn/pull/12?
* Why was `run-all-publication-cases.sh` deleted? We have updated it in the master branch and it's the way to run the whole experiment at once.
Will restore it. I have seen no value in this because the input data is not here? Is this available from a public resource, e.g. zeonodo?
I guess that's all from the top of my head.
Thanks for the review.
@bernt-matthias Here is the solution to the weird sign key error:
@bernt-matthias Here is the solution to the weird sign key error:
Interesting. Still think that it would be best to create a Dockerfile on the basis of https://github.com/tensorflow/tensorflow/blob/master/tensorflow/tools/dockerfiles/dockerfiles/gpu.Dockerfile
Guess it should be sufficient to replace this line by the installation of dfpl (then we also don't need to grep -v
in the setup.py).
Will try this now.
Still think that it would be best to create a Dockerfile on the basis of
Yes, but you need to fix the public keys or you won't get far. Try it out and when you get frustrated, look below :)
# Copyright 2019 The TensorFlow Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# ============================================================================
#
# THIS IS A GENERATED DOCKERFILE.
#
# This file was assembled from multiple pieces, whose use is documented
# throughout. Please refer to the TensorFlow dockerfiles documentation
# for more information.
ARG UBUNTU_VERSION=20.04
ARG ARCH=
ARG CUDA=11.2
FROM nvidia/cuda${ARCH:+-$ARCH}:${CUDA}.1-base-ubuntu${UBUNTU_VERSION} as base
# ARCH and CUDA are specified again because the FROM directive resets ARGs
# (but their default value is retained if set previously)
ARG ARCH
ARG CUDA
ARG CUDNN=8.1.0.77-1
ARG CUDNN_MAJOR_VERSION=8
ARG LIB_DIR_PREFIX=x86_64
ARG LIBNVINFER=7.2.2-1
ARG LIBNVINFER_MAJOR_VERSION=7
# Let us install tzdata painlessly
ENV DEBIAN_FRONTEND=noninteractive
# Needed for string substitution
SHELL ["/bin/bash", "-c"]
RUN sh -c 'echo "APT { Get { AllowUnauthenticated \"1\"; }; };" > /etc/apt/apt.conf.d/99allow_unauth'
RUN apt -o Acquire::AllowInsecureRepositories=true -o Acquire::AllowDowngradeToInsecureRepositories=true update
RUN apt-get install -y curl wget
RUN apt-key del 7fa2af80
RUN wget https://developer.download.nvidia.com/compute/cuda/repos/ubuntu2004/x86_64/cuda-keyring_1.0-1_all.deb
RUN dpkg -i cuda-keyring_1.0-1_all.deb
RUN rm -f /etc/apt/sources.list.d/cuda.list /etc/apt/apt.conf.d/99allow_unauth cuda-keyring_1.0-1_all.deb
RUN apt-key adv --keyserver keyserver.ubuntu.com --recv-keys A4B469963BF863CC F60F4B3D7FA2AF80
# Pick up some TF dependencies
RUN apt-get update && apt-get install -y --no-install-recommends \
build-essential \
cuda-command-line-tools-${CUDA/./-} \
libcublas-${CUDA/./-} \
cuda-nvrtc-${CUDA/./-} \
libcufft-${CUDA/./-} \
libcurand-${CUDA/./-} \
libcusolver-${CUDA/./-} \
libcusparse-${CUDA/./-} \
curl \
libcudnn8=${CUDNN}+cuda${CUDA} \
libfreetype6-dev \
libhdf5-serial-dev \
libzmq3-dev \
pkg-config \
software-properties-common \
unzip
# The rest needs to be fixed...
Super. Container is built https://gitlab.hzdr.de/department-computational-biology/deepfplearn/-/jobs/586680
But tests fail (see log). Hope you can fix this..
Try it out and when you get frustrated, look below :)
Thanks. Was really close to get super frustrated.
In order to make this work
Settings -> Secrets -> Create new Secret
addHZDR_ACCESS_TOKEN
(I will send the token to @mai00fti on another channel)This PR implements the following:
vx.x.x
),dfpl --help
at the moment), andlatest
tag in the registryThe changes to
setup.py
will lead to the creation of/bin/dfpl
on installation, so running withpython -m dfpl
is not necessary anymore (but still possible).Note there are two changes in python scripts that appeared necessary to me. Please review particularly careful.
From now on releases should be done as follows:
setup.py
git commit setup.py -m 'release vx.x.x'
git push upstream master
On the github page a new release can be created. For this create a new version tag (
vx.x.x
) and add sufficient description.TODOs:
README.md
mentionsrun-all-cases.sh
. The data used there seems not available.