recast-hep / recast-atlas

CLI for ATLAS RECAST contributors
https://recast.docs.cern.ch/
Apache License 2.0
5 stars 5 forks source link

Workflow visualization shows only square glyphs in place of titles since v0.1.1 #67

Closed robinnewhouse closed 2 years ago

robinnewhouse commented 2 years ago

In versions 0.1.1 through 0.1.8 (latest), when running recast workflows, the workflow visualization lacks titles for each of the steps and artefacts. In place, square placeholder glyphs are used. It appears that all workflow steps are in the correct places with the expected layout, but there are no titles. If I downgrade to recast 0.1.0, the titles in the workflow return to normal.

This issue appears in the .gif, .png, and .pdf versions of the visualization.

I am running macOS Big Sur 11.6 (20G165)


This can be minimally reproduced with the following:

pip install recast-atlas==0.1.0
recast run examples/rome

which produces _yadage/yadage_workflow_instance.png of

yadage_workflow_instance

compared to

pip install recast-atlas==0.1.1
recast run examples/rome

yadage_workflow_instance-1

matthewfeickert commented 2 years ago

Thanks for the Issue @robinnewhouse. Can you also give us the Python runtime version that you're using in your virtual environment? So the output of

$ python --version --version
robinnewhouse commented 2 years ago

I guess I very rarely use my local python, and I admit, I typically use conda instead of venv. In this instance, I'm using neither and my python version is Python 2.7.16. I'm upgrading to 3 right away ;)

matthewfeickert commented 2 years ago

Hm, I'm assuming from the diff between v0.1.0...v0.1.1 that this is related to a packtivity change given

$ git diff v0.1.0 v0.1.1 -- setup.py
diff --git a/setup.py b/setup.py
index 0e24dc2..7e18883 100644
--- a/setup.py
+++ b/setup.py
@@ -10,7 +10,7 @@ with open(path.join(this_directory, 'README.md'), encoding='utf-8') as readme_md

 setup(
   name = 'recast-atlas',
-  version = '0.1.0',
+  version = '0.1.1',
   description = 'RECAST for ATLAS at the LHC',
   url = '',
   author = 'Lukas Heinrich',
@@ -33,7 +33,7 @@ setup(
       'pydotplus==2.0.2',
       'adage==0.10.1',
       'yadage-schemas==0.10.6',
-      'packtivity==0.14.21',
+      'packtivity==0.14.23',
       'yadage[viz]==0.20.1',
     ],
     'kubernetes': [

But I haven't looked at this really at all and I'm not a packtivity expert (or even novice). So maybe @lukasheinrich can say if that's a reasonable thing at all or not.

matthewfeickert commented 2 years ago

In this instance, I'm using neither and my python version is Python 2.7.16.

Okay thanks. I ask only so that I can try to replicate your working environment when debugging with you.

I guess I very rarely use my local python, and I admit, I typically use conda instead of venv

That's fine. I'm using "virtual environment" here to mean it in the most broad sense (so a venv env and a Conda env both count)

I'm upgrading to 3 right away ;)

:snake: recast-atlas has CI covering Python 3.6 - Python 3.8 (and should have Python 3.9 and Python 3.10 support soon) so I think you should be good to go there as long as you're in the currently supported range.

matthewfeickert commented 2 years ago

I can reproduce this in Python 3.8.11 environments, but given that I installed recast-atlas from PyPI in two different virtual environments (one recast-atlas v0.1.0 and one recast-atlas v0.1.1) and

$ diff freeze-recast-v0.1.0.txt freeze-recast-v0.1.1.txt 
10c10
< recast-atlas==0.1.0
---
> recast-atlas==0.1.1

I'm pretty sure the difference is coming from the Docker images (as the default backend is the Docker backend). There's very minor changes in the Dockerfile itself

$ git diff v0.1.0 v0.1.1 -- docker/Dockerfile
diff --git a/docker/Dockerfile b/docker/Dockerfile
index b11a1ad..dc69998 100644
--- a/docker/Dockerfile
+++ b/docker/Dockerfile
@@ -1,12 +1,13 @@
-FROM docker:19.03.0
+FROM docker:20.10.5
 RUN  apk add py-pip automake autoconf libtool \
-             python-dev musl-dev libffi-dev tree \
-             python-dev musl-dev libffi-dev gcc \
+             python3 python3-dev musl-dev libffi-dev tree \
+             python3-dev musl-dev libffi-dev gcc \
              autoconf curl gcc ipset ipset-dev iptables iptables-dev libnfnetlink libnfnetlink-dev libnl3 libnl3-dev make musl-dev openssl openssl-dev \
-             jq util-linux font-bitstream-type1 build-base graphviz-dev imagemagick graphviz
+             jq util-linux font-bitstream-type1 build-base graphviz-dev imagemagick graphviz gcc musl-dev python3-dev libffi-dev openssl-dev cargo
+
 WORKDIR /code
 COPY . /code
-RUN pip install -e .[local,kubernetes]
+RUN pip3 install -e .[local,kubernetes]
 ENV PACKTIVITY_DOCKER_CMD_MOD "-u root"
 ENV PACKTIVITY_CVMFS_LOCATION /shared-mounts/cvmfs
 ENV PACKTIVITY_CVMFS_PROPAGATION rslave

but we see (that in addition to a Python 2.7 to Python 3.7 change) there's a lot of differences in Python packages.

$ docker run --rm recast/recastatlas:v0.1.0 /bin/ash -c 'python -m pip freeze' &> freeze_docker_v0.1.0.txt 
$ docker run --rm recast/recastatlas:v0.1.1 /bin/ash -c 'python3 -m pip freeze' &> freeze_docker_v0.1.1.txt
$ diff -y freeze_docker_v0.1.0.txt freeze_docker_v0.1.1.txt | wc -l
66

The pip freezes are attached here:

and their diff

diff -y of pip freezes: ```console $ diff -y freeze_docker_v0.1.0.txt freeze_docker_v0.1.1.txt Error [Errno 2] No such file or directory while executing com < adage==0.10.1 adage==0.10.1 attrs==19.3.0 | appdirs==1.4.4 cachetools==3.1.1 | attrs==20.3.0 certifi==2019.11.28 | CacheControl==0.12.6 cffi==1.14.0 | cachetools==4.2.1 chardet==3.0.4 | certifi==2020.12.5 checksumdir==1.1.7 | cffi==1.14.5 Click==7.0 | chardet==4.0.0 configparser==4.0.2 | checksumdir==1.2.0 contextlib2==0.6.0.post1 | click==7.1.2 cryptography==2.8 | colorama==0.4.4 decorator==4.4.1 | contextlib2==0.6.0 enum34==1.1.6 | cryptography==3.4.7 funcsigs==1.0.2 | decorator==5.0.7 functools32==3.2.3.post2 | distlib==0.3.1 > distro==1.5.0 glob2==0.7 glob2==0.7 google-auth==1.11.0 | google-auth==1.30.0 idna==2.8 | html5lib==1.1 importlib-metadata==1.5.0 | idna==3.1 ipaddress==1.0.23 | jq==1.1.2 jq==0.1.6 < jsonpath-rw==1.4.0 jsonpath-rw==1.4.0 jsonpointer==2.0 | jsonpointer==2.1 jsonref==0.2 jsonref==0.2 jsonschema==3.2.0 jsonschema==3.2.0 kubernetes==9.0.0 kubernetes==9.0.0 mock==3.0.5 | lockfile==0.12.2 > mock==4.0.3 > msgpack==1.0.2 networkx==1.11 networkx==1.11 oauthlib==3.1.0 oauthlib==3.1.0 packtivity==0.14.21 | ordered-set==4.0.2 pathlib2==2.3.5 | packaging==20.9 > packtivity==0.14.23 > pep517==0.9.1 ply==3.11 ply==3.11 psutil==5.6.7 | progress==1.5 > psutil==5.8.0 pyasn1==0.4.8 pyasn1==0.4.8 pyasn1-modules==0.2.8 pyasn1-modules==0.2.8 pycparser==2.19 | pycparser==2.20 pydot2==1.0.33 pydot2==1.0.33 pydotplus==2.0.2 pydotplus==2.0.2 pygraphviz==1.5 | pygraphviz==1.7 pyOpenSSL==19.1.0 | pyOpenSSL==20.0.1 pyparsing==2.4.6 | pyparsing==2.4.7 pyrsistent==0.15.7 | pyrsistent==0.17.3 python-dateutil==2.8.1 python-dateutil==2.8.1 PyYAML==5.3 | pytoml==0.1.21 recast-atlas==0.1.0 | PyYAML==5.4.1 requests==2.22.0 | # Editable install with no version control (recast-atlas==0.1 > -e /code/src > requests==2.25.1 requests-oauthlib==1.3.0 requests-oauthlib==1.3.0 rsa==4.0 | retrying==1.3.3 scandir==1.10.0 | rsa==4.7.2 six==1.14.0 | six==1.15.0 urllib3==1.25.8 | toml==0.10.2 websocket-client==0.57.0 | urllib3==1.26.2 > webencodings==0.5.1 > websocket-client==0.58.0 yadage==0.20.1 yadage==0.20.1 yadage-schemas==0.10.6 yadage-schemas==0.10.6 zipp==1.1.0 < ```

I think we're going to need @lukasheinrich's advice here as I'm not really sure what the graph dependencies are of yadage that is actually doing the drawing here, though it apparently isn't the fault of networkx or pydot2

$ diff freeze_docker_v0.1.0.txt freeze_docker_v0.1.1.txt | grep 'networkx\|pydot'  # Returns nothing
lukasheinrich commented 2 years ago

hm as a suggestion can you run somethting simple through graphviz

e.g.

strict digraph {
   A -> B;
}

dot -Tpdf test.pdf ? That might help debug it

lukasheinrich commented 2 years ago

mayb it's som libgraphviz related issue

matthewfeickert commented 2 years ago

That might help debug it

Indeed.

$ docker pull recast/recastatlas:v0.1.8
$ docker run --rm -ti -v $PWD:/work recast/recastatlas:v0.1.8
/work # cat test.gv 
strict digraph {
   A -> B;
}
/work # dot -Tpdf test.gv -o test.pdf
/work # dot -Tpng test.gv -o test.png

produces this test.png

test

I have a branch (docker/update-dockerfile) that is trying to get a python:3.8-slim-bullseye based image working which is able to render this correctly

$ docker run --rm -ti -v $PWD:/work recast/recastatlas:local-debug 
root@d5e1c4970e41:/work# cat /etc/os-release 
PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"
NAME="Debian GNU/Linux"
VERSION_ID="11"
VERSION="11 (bullseye)"
VERSION_CODENAME=bullseye
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"
root@d5e1c4970e41:/work# cat test.gv 
strict digraph {
   A -> B;
}
root@d5e1c4970e41:/work# dot -Tpdf test.gv -o test.pdf
root@d5e1c4970e41:/work# dot -Tpng test.gv -o test.png
root@d5e1c4970e41:/work# 

test_working

but I need to figure out how to get docker working in this image for it to be useful.

(Related: Issue #50)

lukasheinrich commented 2 years ago

this is what yadag runs, so if dot works yadag viz should also work:

https://github.com/yadage/yadage/blob/cbb26515f02265800b4f5156e6c099211121d6a8/yadage/visualize.py#L121

matthewfeickert commented 2 years ago

if dot works yadag viz should also work:

Cool. This is good to know so we only have 1 thing to debug.

matthewfeickert commented 2 years ago

Here's a minimal working Dockerfile

FROM docker:20.10.5

RUN apk add --update && \
    apk add --no-cache \
        graphviz \
        ttf-freefont

WORKDIR /work

that when built

docker build -f Dockerfile -t graphviz:debug-local .

is able to produce the minimal example

$ docker run --rm -v $PWD:/work graphviz:debug-local /bin/ash -c 'dot -Tpng test.gv -o test.png' 

test

So I think ttf-freefont is the only thing missing from the current Dockerfile.

matthewfeickert commented 2 years ago

Yeah, it is just ttf-freefont. With

FROM recast/recastatlas:v0.1.8

RUN apk add --update && \
    apk add --no-cache \
        graphviz \
        ttf-freefont

I can get

#!/bin/bash

docker pull recast/recastatlas:v0.1.8
docker tag recast/recastatlas:v0.1.8 recast/recastatlas:v0.1.8-build-base
docker build -f Dockerfile -t recast/recastatlas:v0.1.8 .

docker run --rm -v $PWD:/work recast/recastatlas:v0.1.8 /bin/ash -c 'dot -Tpng test.gv -o test.png'

to produce the file as expected and then, as this new images is tagged to match the release pattern recast is looking for

$ recast run examples/rome --tag issue-67

produces the following recast-issue-67/_yadage/yadage_workflow_instance.png

yadage_workflow_instance

:+1:

I'll open a PR to fix this, and we can backport this to other images too.

matthewfeickert commented 2 years ago

@robinnewhouse I'll ping you here once I have the change from PR #69 backported to the recast/recastatlas:v0.1.8 Docker image.

robinnewhouse commented 2 years ago

Wow, you guys are fast! Thanks for the fix :)

matthewfeickert commented 2 years ago

@robinnewhouse I've backported the fix to recast/recastatlas:v0.1.8 so if you repull you should be good to go. :+1:

$ docker pull recast/recastatlas:v0.1.8
$ python -m pip list | grep recast-atlas
recast-atlas       0.1.8
$ recast run examples/rome --tag rome-example

produces this recast-rome-example/_yadage/yadage_workflow_instance.png

yadage_workflow_instance

Thanks for opening up a great Issue and if you come across other bugs please do so as well — we really appreciate it!