thaler-lab / Wasserstein

Python/C++ library for computing Wasserstein distances efficiently.
https://thaler-lab.github.io/Wasserstein
Other
21 stars 8 forks source link

fix: Use signed char to avoid compiler error #5

Closed matthewfeickert closed 1 year ago

matthewfeickert commented 1 year ago

Under arm64 architectures the use of a negative char results in an error of

error: enumerator value ‘-1’ is outside the range of underlying type ‘char’

At the request of the project maintainer, use a signed char instead of changing the values of the enum.

matthewfeickert commented 1 year ago

@pkomiske this is ready for review. This PR's branch builds under an arm64 architecture as demonstrated by the following Dockerfile and Makefile

ARG TARGETARCH

USER root

SHELL [ "/bin/bash", "-c" ]

COPY . /code WORKDIR /code

Set PATH to pickup virtualenv by default

ENV PATH=/usr/local/venv/bin:"${PATH}" RUN apt-get -qq -y update && \ apt-get -qq -y install --no-install-recommends \ gcc \ g++ \ make \ cmake \ python3-dev \ git && \ apt-get -y clean && \ apt-get -y autoremove && \ rm -rf /var/lib/apt/lists/* && \ python -m venv /usr/local/venv && \ . /usr/local/venv/bin/activate && \ python -m pip --no-cache-dir install --upgrade \ pip \ setuptools \ wheel && \ python -m pip --no-cache-dir install . && \ python -m pip list && \ printf '\nexport PATH=/usr/local/venv/bin:"${PATH}"\n' >> /root/.bashrc

ENV PYTHONPATH=/usr/local/venv/lib:${PYTHONPATH} ENV LD_LIBRARY_PATH=/usr/local/venv/lib:${LD_LIBRARY_PATH}

WORKDIR /


* `Makefile`

```makefile
default: arm64

all: arm64

arm64:
    docker pull python:3.10-slim-bullseye
    docker buildx create \
        --name buildx_builder \
        --driver docker-container \
        --bootstrap \
        --use
    docker buildx build \
        --file docker/Dockerfile \
        --platform linux/arm64 \
        --build-arg BUILDER_IMAGE=python:3.10-slim-bullseye \
        --tag ghcr.io/pkomiske/wasserstein:debug-arm64 \
        --load \
        .
    docker buildx stop buildx_builder
    docker buildx rm buildx_builder
$ make
$ docker images ghcr.io/pkomiske/wasserstein:debug-arm64
REPOSITORY                     TAG           IMAGE ID       CREATED          SIZE
ghcr.io/pkomiske/wasserstein   debug-arm64   5332bd83ebcb   14 minutes ago   607MB
$ docker image inspect ghcr.io/pkomiske/wasserstein:debug-arm64 | jq '.[] | with_entries(select(.key == "Architecture"))'
{
  "Architecture": "arm64"
}
pkomiske commented 1 year ago

Thanks! I'll approve this but I may not be able to get new versions uploaded to PyPI right away.

matthewfeickert commented 1 year ago

I'll approve this but I may not be able to get new versions uploaded to PyPI right away.

Thanks! That's fine that you might not get to it right off the bat. I know that you already have the Wasserstein-1.1.0-*-macosx_11_0_arm64.whl wheels up for Mac, but as I was hitting Issue #3 while trying to build Docker images with wasserstein v1.1.0 in them that had both amd64 and arm64 manifests so that M1 users could not get hit with super slow emulation at runtime. So whenever you can get a v1.1.1 patch release up is great. :+1:

matthewfeickert commented 1 year ago

@pkomiske Just wanted to follow up to see if you think a patch release before 2023 is realistic or not — I'm asking not to try and bug you, but more to have some rough idea of timelines. :+1:

matthewfeickert commented 1 year ago

@pkomiske :wave: Would it be possible to get a new release with wheels on PyPI in February of 2023?

matthewfeickert commented 1 year ago

@pkomiske :wave: Would it be possible to get a new release with wheels on PyPI before March of 2023?

matthewfeickert commented 1 year ago

@pkomiske Would you be willing to make a patch release? It would be great to get Python 3.11 wheels as well.

100rab-S commented 11 months ago

Hey @pkomiske, it would be great if we can get the updated package on PyPI. Please!!

matthewfeickert commented 11 months ago

@100rab-S We should maybe note that @pkomiske hasn't had any public GitHub activity since November, 2022 and attempt to reach him through some other communication channel.

matthewfeickert commented 8 months ago

@rikab now that this has gotten moved over to the thaler-lab GitHub org, could we work together to make a v1.1.1 release? (c.f. https://github.com/thaler-lab/Wasserstein/pull/5#issuecomment-1312059614).