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

Fix rastervision_pipeline entry point to ensure commands from other plugins are available #1250

Merged
merged 1 commit into from
Aug 30, 2021

Conversation

AdeelH
Copy link
Collaborator

@AdeelH AdeelH commented Aug 19, 2021

Overview

This PR fixes the problem of the predict command not being available if RV is installed via setup.y. See #1246 for a detailed discussion. In summary, the problem was caused by registry.get_plugin_commands() being called before the plugins had been loaded.

Checklist

  • Added needs-backport label if PR is bug fix that applies to previous minor release
  • Ran scripts/format_code and committed any changes
  • Documentation updated if needed
  • PR has a name that won't get you publicly shamed for vagueness

Testing Instructions

Create a docker image, with RV installed vai pip, using the Dockerfile below.

Dockerfile
FROM nvidia/cuda:10.2-cudnn7-devel-ubuntu16.04

RUN apt-get update && apt-get install -y software-properties-common python-software-properties
RUN add-apt-repository ppa:ubuntugis/ppa && \
	apt-get update && \
	apt-get install -y wget=1.* git=1:2.* python-protobuf=2.* python3-tk=3.* \
	jq=1.5* \
	build-essential libsqlite3-dev=3.11.* zlib1g-dev=1:1.2.* \
	libopencv-dev=2.4.* python-opencv=2.4.* unzip curl && \
	apt-get autoremove && apt-get autoclean && apt-get clean

# See https://github.com/mapbox/rasterio/issues/1289
ENV CURL_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt

# Install Python 3.6
RUN wget -q -O ~/miniconda.sh https://repo.anaconda.com/miniconda/Miniconda3-4.7.12.1-Linux-x86_64.sh && \
	chmod +x ~/miniconda.sh && \
	~/miniconda.sh -b -p /opt/conda && \
	rm ~/miniconda.sh
ENV PATH /opt/conda/bin:$PATH
ENV LD_LIBRARY_PATH /opt/conda/lib/:$LD_LIBRARY_PATH
RUN conda install -y python=3.6
RUN python -m pip install --upgrade pip
RUN conda install -y -c conda-forge gdal=3.0.4

# Setup GDAL_DATA directory, rasterio needs it.
ENV GDAL_DATA=/opt/conda/lib/python3.6/site-packages/rasterio/gdal_data/

WORKDIR /opt/src/
COPY ./raster-vision/ /opt/src/raster-vision

RUN pip install rastervision==0.13

# Needed for click to work
ENV LC_ALL C.UTF-8
ENV LANG C.UTF-8
ENV PROJ_LIB /opt/conda/share/proj/

CMD ["bash"]
DOCKER_BUILDKIT=1 docker build -t rvtest -f Dockerfile .

docker run --gpus=all --rm --ipc=host -it rvtest

Verify that the problem is caused by rastervision-aws-batch

rastervision
# Usage: rastervision [OPTIONS] COMMAND [ARGS]...

#   The main click command.

#   Sets the profile, verbosity, and tmp_dir in RVConfig.

# Options:
#   -p, --profile TEXT  Sets the configuration profile name to use.
#   -v, --verbose       Increment the verbosity level.
#   --tmpdir TEXT       Root of temporary directories to use.
#   --help              Show this message and exit.

# Commands:
#   run          Run sequence of commands within pipeline(s).
#   run_command  Run an individual command within a pipeline.

pip uninstall rastervision-aws-batch -y

rastervision
# Usage: rastervision [OPTIONS] COMMAND [ARGS]...

#   The main click command.

#   Sets the profile, verbosity, and tmp_dir in RVConfig.

# Options:
#   -p, --profile TEXT  Sets the configuration profile name to use.
#   -v, --verbose       Increment the verbosity level.
#   --tmpdir TEXT       Root of temporary directories to use.
#   --help              Show this message and exit.

# Commands:
#   predict      Use a model bundle to predict on new images.
#   run          Run sequence of commands within pipeline(s).
#   run_command  Run an individual command within a pipeline.

pip install rastervision-aws-batch

rastervision
# Usage: rastervision [OPTIONS] COMMAND [ARGS]...

#   The main click command.

#   Sets the profile, verbosity, and tmp_dir in RVConfig.

# Options:
#   -p, --profile TEXT  Sets the configuration profile name to use.
#   -v, --verbose       Increment the verbosity level.
#   --tmpdir TEXT       Root of temporary directories to use.
#   --help              Show this message and exit.

# Commands:
#   run          Run sequence of commands within pipeline(s).
#   run_command  Run an individual command within a pipeline.

Test PR

git clone https://github.com/azavea/raster-vision.git
cd raster-vision/
git fetch https://github.com/AdeelH/raster-vision.git cli_predict
git checkout FETCH_HEAD
pip install -e rastervision_pipeline/ --upgrade

rastervision
# Usage: rastervision [OPTIONS] COMMAND [ARGS]...

#   The main click command.

#   Sets the profile, verbosity, and tmp_dir in RVConfig.

# Options:
#   -p, --profile TEXT  Sets the configuration profile name to use.
#   -v, --verbose       Increment the verbosity level.  [x>=0]
#   --tmpdir TEXT       Root of temporary directories to use.
#   --help              Show this message and exit.

# Commands:
#   predict      Use a model bundle to predict on new images.
#   run          Run sequence of commands within pipeline(s).
#   run_command  Run an individual command within a pipeline.

Closes #1246

"All problems in computer science can be solved by another level of indirection"
@codecov
Copy link

codecov bot commented Aug 20, 2021

Codecov Report

Merging #1250 (2831b3a) into master (4eaa709) will decrease coverage by 0.02%.
The diff coverage is 20.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1250      +/-   ##
==========================================
- Coverage   57.97%   57.94%   -0.03%     
==========================================
  Files         168      168              
  Lines        7636     7638       +2     
==========================================
- Hits         4427     4426       -1     
- Misses       3209     3212       +3     
Impacted Files Coverage Δ
rastervision_pipeline/rastervision/pipeline/cli.py 36.03% <20.00%> (-0.67%) ⬇️
...rvision_pipeline/rastervision/pipeline/registry.py 85.18% <0.00%> (-0.93%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4eaa709...2831b3a. Read the comment docs.

@AdeelH AdeelH merged commit 00b2ecb into azavea:master Aug 30, 2021
@AdeelH AdeelH deleted the cli_predict branch August 30, 2021 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ignored tmpdir option, missing predict command after native installation on Ubuntu 16.04
2 participants