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

Replace conda with pyenv to fix incorrect libstdc++ use in jammy CI. #6966

Merged
merged 6 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 6 additions & 19 deletions cpp/open3d/core/nns/NanoFlannImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,6 @@ void _KnnSearchCPU(NanoFlannIndexHolderBase *holder,
return;
}

auto points_equal = [](const T *const p1, const T *const p2,
size_t dimension) {
std::vector<T> p1_vec(p1, p1 + dimension);
std::vector<T> p2_vec(p2, p2 + dimension);
return p1_vec == p2_vec;
};

std::vector<std::vector<TIndex>> neighbors_indices(num_queries);
std::vector<std::vector<T>> neighbors_distances(num_queries);
std::vector<uint32_t> neighbors_count(num_queries, 0);
Expand All @@ -147,8 +140,9 @@ void _KnnSearchCPU(NanoFlannIndexHolderBase *holder,
for (size_t valid_i = 0; valid_i < num_valid; ++valid_i) {
TIndex idx = result_indices[valid_i];
if (ignore_query_point &&
points_equal(&queries[i * dimension],
&points[idx * dimension], dimension)) {
std::equal(&queries[i * dimension],
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch! This should significantly reduce the number of heap allocations

&queries[i * dimension] + dimension,
&points[idx * dimension])) {
continue;
}
neighbors_indices[i].push_back(idx);
Expand Down Expand Up @@ -222,13 +216,6 @@ void _RadiusSearchCPU(NanoFlannIndexHolderBase *holder,
return;
}

auto points_equal = [](const T *const p1, const T *const p2,
size_t dimension) {
std::vector<T> p1_vec(p1, p1 + dimension);
std::vector<T> p2_vec(p2, p2 + dimension);
return p1_vec == p2_vec;
};

std::vector<std::vector<TIndex>> neighbors_indices(num_queries);
std::vector<std::vector<T>> neighbors_distances(num_queries);
std::vector<uint32_t> neighbors_count(num_queries, 0);
Expand All @@ -255,9 +242,9 @@ void _RadiusSearchCPU(NanoFlannIndexHolderBase *holder,
int num_neighbors = 0;
for (const auto &idx_dist : search_result) {
if (ignore_query_point &&
points_equal(&queries[i * dimension],
&points[idx_dist.first * dimension],
dimension)) {
std::equal(&queries[i * dimension],
&queries[i * dimension] + dimension,
&points[idx_dist.first * dimension])) {
continue;
}
neighbors_indices[i].push_back(idx_dist.first);
Expand Down
7 changes: 7 additions & 0 deletions cpp/tests/t/geometry/TensorMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ TEST_P(TensorMapPermuteDevices, Constructor) {
// Primary key is required.
EXPECT_ANY_THROW(t::geometry::TensorMap());

// Delete primary key.
EXPECT_ANY_THROW(tm0.Erase("positions"));

// Reserved keys.
EXPECT_ANY_THROW(tm0.insert(
{"primary_key", core::Tensor::Zeros({2, 3}, dtype, device)}));

// Iterators.
std::map<std::string, core::Tensor> tensor_map(
{{"positions", core::Tensor::Zeros({10, 3}, dtype, device)},
Expand Down
51 changes: 36 additions & 15 deletions docker/Dockerfile.ci
Original file line number Diff line number Diff line change
Expand Up @@ -67,34 +67,55 @@ RUN if [ "${BUILD_SYCL_MODULE}" = "ON" ]; then \
rm -rf /etc/apt/sources.list.d/oneAPI.list; \
fi

# Dependencies: basic
# Dependencies: basic and python-build
# gcc-11 causes a seg fault in tensormap when built in Release mode. Upgrade to
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we found the root cause for that or is it really a bug only due to gcc11?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching that. I think it was actually the libunwind issue. I've been working with gcc-11 on jammy (22.04) without issues. Reverted.

# gcc-13 instead.
RUN apt-get update && apt-get install -y \
git \
wget \
curl \
build-essential \
pkg-config \
zlib1g \
zlib1g-dev \
libssl-dev \
libbz2-dev \
libreadline-dev \
libsqlite3-dev \
libncursesw5-dev \
xz-utils \
tk-dev \
libxml2-dev \
libxmlsec1-dev \
libffi-dev \
liblzma-dev \
&& if c++ --version | grep -F ' 11.' ; then \
apt-get install -y g++-13 \
&& update-alternatives --install /usr/bin/c++ c++ /usr/bin/g++-13 100 \
&& c++ --version; \
fi \
&& rm -rf /var/lib/apt/lists/*

# Miniconda or Intel conda
# The **/open3d/bin paths are used during docker run, in this way docker run
# pyenv or Intel Python
# The pyenv python paths are used during docker run, in this way docker run
# does not need to activate the environment again.
ENV PATH="/root/miniconda3/bin:${PATH}"
ENV PATH="/root/miniconda3/envs/open3d/bin:${PATH}"
# The soft link from the python patch level version to the python mino version
# ensures python wheel commands (i.e. open3d) are in PATH, since we don't know
# which patch level pyenv will install (latest).
ENV PYENV_ROOT=/root/.pyenv
ENV PATH="$PYENV_ROOT/shims:$PYENV_ROOT/bin:$PYENV_ROOT/versions/$PYTHON_VERSION/bin:$PATH"
ENV PATH="/opt/intel/oneapi/intelpython/latest/bin:${PATH}"
ENV PATH="/opt/intel/oneapi/intelpython/latest/envs/open3d/bin:${PATH}"
RUN if [ "${BUILD_SYCL_MODULE}" = "OFF" ]; then \
wget -q https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh; \
bash Miniconda3-latest-Linux-x86_64.sh -b; \
rm Miniconda3-latest-Linux-x86_64.sh; \
curl https://pyenv.run | bash \
&& pyenv update \
Copy link
Contributor

Choose a reason for hiding this comment

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

I was working on switching to Miniforge in #6717 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

miniforge likely won't fix this issue, since it's a copy of conda and likely uses it's on libstdc++ as well.

&& pyenv install $PYTHON_VERSION \
&& pyenv global $PYTHON_VERSION \
&& pyenv rehash \
&& ln -s $PYENV_ROOT/versions/${PYTHON_VERSION}* $PYENV_ROOT/versions/${PYTHON_VERSION}; \
fi
RUN conda --version \
&& conda create -y -n open3d python=${PYTHON_VERSION}
RUN python --version && pip --version

# Activate open3d virtualenv
# This works during docker build. It becomes the prefix of all RUN commands.
# Ref: https://stackoverflow.com/a/60148365/1255535
SHELL ["conda", "run", "-n", "open3d", "/bin/bash", "-o", "pipefail", "-c"]
SHELL ["/bin/bash", "-o", "pipefail", "-c"]

# Dependencies: cmake
ENV PATH=${HOME}/${CMAKE_VERSION}/bin:${PATH}
Expand Down
2 changes: 1 addition & 1 deletion docker/docker_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ cpp_python_linking_uninstall_test() {
# Python test
echo "pytest is randomized, add --randomly-seed=SEED to repeat the test sequence."
${docker_run} -i --rm "${DOCKER_TAG}" /bin/bash -c " \
python -m pytest python/test ${pytest_args} -s"
python -W default -m pytest python/test ${pytest_args} -s"
restart_docker_daemon_if_on_gcloud

# Command-line tools test
Expand Down
Loading