ufal / nametag

NameTag: Named Entity Tagger
Mozilla Public License 2.0
38 stars 10 forks source link

Invalid and incorrect JSON responses for some Python runs for Py 3.5 and lower [nametag2] #19

Closed PROrock closed 2 years ago

PROrock commented 2 years ago

If using Python 3.5 (e.g. using the Dockerfile), for some runs of the nametag2 (REST server mode) it returns the response JSON with attributes in a different order than "model, acknowledgements, result". In case this happens the result is an incorrect response and sometimes even an invalid JSON (see examples below). When I restart the container with the nametag server (so it stops and starts the NameTag2 server), there is a different order of the attributes in the JSON response. If the order is truly random it means that in 2/3 of cases it has the wrong order and incorrect JSON and in 1/3 of cases, it has incorrect AND invalid JSON.

Curl to test it: curl --location --request GET 'localhost:8001/recognize?data=Ondra&output=vertical'

Result from one run of the NameTag2 server (the correct one):

{
 "acknowledgements": [
  "http://ufal.mff.cuni.cz/nametag/2#acknowledgements",
  "ack-text"
 ],
 "result": "",
 "model": "czech-cnec2.0-2008311\tgu\tOndra\n"
}

Result from another run of the NameTag2 server (the incorrect one):

{
 "acknowledgements": [
  "http://ufal.mff.cuni.cz/nametag/2#acknowledgements",
  "ack-text"
 ],
 "model": "czech-cnec2.0-200831",
 "result": "1\tgu\tOndra\n"
}

Result from yet another run of the NameTag2 server (the incorrect and invalid one):

{
 "result": "",
 "model": "czech-cnec2.0-200831",
 "acknowledgements": [
  "http://ufal.mff.cuni.cz/nametag/2#acknowledgements",
  "ack-text"
 1\tgu\tOndra\n"
}

My environment: OS: MacOS 11.6.4 Container engine: Podman 3.4 using Dockerfile in branch nametag2 https://github.com/ufal/nametag/blob/nametag2/Dockerfile Python version in the image: Python 3.5.2 (affected probably 3.5 and lower)

I have tested this also outside of the Podman/container with Python 3.7.9 and I cannot replicate this bug there (further supporting my explanation below).

My understanding of the issue: Based on https://stackoverflow.com/questions/39980323/are-dictionaries-ordered-in-python-3-6 before python 3.6 (i.e. 3.5 and lower) the dict has not guaranteed the order of iteration. But since 3.6 and newer the Python dict is insertion ordered, so it keeps the order in which the attributes were given to it.

My non-exhaustive list of possible solutions:

Note: I'm a Czech and we can continue in Czech if you would prefer it that way :-)

PROrock commented 2 years ago

I have a workaround for this with my modified Dockerfile (the image has half the size of the original!):

FROM python:3.7-slim as builder

RUN apt-get update -qq && \
  apt-get install -y --no-install-recommends \
  # needed for gcc and other c-compilation things (for tensorflow)
  build-essential \
  && rm -rf /var/lib/apt/lists/*

ENV VIRTUAL_ENV=/opt/venv
RUN python -m venv $VIRTUAL_ENV
# make sure we use the virtualenv. This is instead of "activate"
ENV PATH="$VIRTUAL_ENV/bin:$PATH"

COPY requirements.txt requirements.txt
RUN pip install --no-cache-dir --upgrade pip -r requirements.txt

FROM python:3.7-slim
EXPOSE 8001
# This prevents Python from writing out pyc files
ENV PYTHONDONTWRITEBYTECODE=1

RUN useradd --create-home appuser
USER appuser

ENV VIRTUAL_ENV=/opt/venv
# copy everything from builder's VIRTUAL_ENV
COPY --from=builder $VIRTUAL_ENV $VIRTUAL_ENV
# make sure we use the virtualenv. This is instead of "activate"
ENV PATH="$VIRTUAL_ENV/bin:$PATH"

WORKDIR /app
COPY . .

ENTRYPOINT ["python", "nametag2_server.py"]
CMD ["8001", "host.containers.internal:8000", \
     "czech-cnec2.0-200831", "czech-cnec2.0-200831", "nametag2-models-200831/czech-cnec2.0-200831/", "ack-text"]

But you have to download the model zip before building the Docker image (or install the curl + unzip tools in the Dockerfile, before using them).

foxik commented 2 years ago

Hi,

sorry it took so long, but we had Covid in the family (everyone is fine now, though :-).

The reason for the incremental construction of the JSON is to keep the HTTP connection active -- this way, even if the prediction takes several minutes, we steadily send data, which keeps the connection open.

Python 3.5 was end-of-life more than half a year before our initial release, so we did not bother supporting it. But given that is is still useful, we can easily use an OrderedDict -- I just pushed it in 607ff41.

PROrock commented 2 years ago

Hi, foxik.

I'm glad you and your family are well :-)

Thanks for the OrderedDict fix. Also, thanks for the explanation of the incremental construction of the JSON. Maybe this could be documented in a comment in the code.

One last thing, you say that Python 3.5 was obsolete even before you start this project, but you use it in the Dockerfile, as the base image tensorflow/tensorflow:1.12.3-py3 uses Python 3.5.2. Do you know about this? Are you planning to fix this? You can get inspired by my Dockerfile higher in the comments for a solution.

foxik commented 2 years ago

Hi,

thanks for the note about the Python 3.5 in the dockerfile, that is definitely an error. I just increased the TF version to the latest version in the 1 series, which has Python 3.6.9. I also updated the Dockerfile a bit to download all the latest models and set a CMD which would load them all (but the wembedding server must still be manually specified) in 380d7fb.