making ipython optional

[hmaarrfk]

Would you consider making ipython an optional dependency?

Patch below, i can make a PR if you like

the patch

From 46ecf8e079ecc25c2e62455a8ca9ae17840813ec Mon Sep 17 00:00:00 2001
From: Mark Harfouche <[email protected]>
Date: Sun, 25 Jun 2023 13:09:54 -0400
Subject: [PATCH 3/3] Make IPython an optional dependency

---
 bindings/cython/datoviz/__init__.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/bindings/cython/datoviz/__init__.py b/bindings/cython/datoviz/__init__.py
index 94da4475..6d8c832f 100644
--- a/bindings/cython/datoviz/__init__.py
+++ b/bindings/cython/datoviz/__init__.py
@@ -7,9 +7,6 @@ import sys
 import time
 import __main__ as main
 
-from IPython import get_ipython
-from IPython.terminal.pt_inputhooks import register
-
 try:
     from .pydatoviz import App, colormap, colorpal, demo
 except ImportError:
@@ -113,6 +110,7 @@ def inputhook(context):
 
 
 def enable_ipython():
+    from IPython import get_ipython
     ipython = get_ipython()
     if ipython:
         logger.info("Enabling Datoviz IPython event loop integration")
@@ -139,7 +137,12 @@ def is_interactive():
 
 
 # print(f"In IPython: {in_ipython()}, is interactive: {is_interactive()}")
-register('datoviz', inputhook)
+try:
+    from IPython.terminal.pt_inputhooks import register
+
+    register('datoviz', inputhook)
+except ImportError:
+    pass
 
 
 # Event loops
-- 
2.39.2

[rossant]

Sure! Yes, I'll happily merge a PR.

[hmaarrfk]

Would it be possible to reopen this?

I don't think that my "fix" actually resolved things for the standard python console in interactive mode.

It seems to trigger this code path https://github.com/datoviz/datoviz/blob/2b8bdd0f03176a4b1f72c43e9c4d777fe102a77e/bindings/cython/datoviz/init.py#L188

    if event_loop == 'ipython' or is_interactive():
        enable_ipython()

which then fails at the import IPython statement.

[rossant]

Wanna make a new PR? 🙏

[hmaarrfk]

truthfully, i'm having trouble getting the python demos to work on my system and i'm trying to not be the annoying user that say:

"it is broken"

when you just told me

"yeah, its a WIP".

I'm on ubuntu 23.04 and it may be causing that too.

I'm happy to test on a few different systems, I have some with NVidia GPUs too if you want.

[rossant]

do you get specific error messages? Are you trying with the system Python or with a conda environment?

[hmaarrfk]

conda-forge (+ some bleeding edge packages that aren't released on conda-forge yet) environment.

[hmaarrfk]

On distributed the error looks like when running test_app_1

10:04:27.278 T0 W          vkutils.h:0245: validation layer support missing, make sure you have exported the environment variable VK_LAYER_PATH="$VULKAN_SDK/etc/vulkan/explicit_layer.d"
10:04:27.278 T0 W          vkutils.h:0318: disable Vulkan validation layer
10:04:27.334 T0 E            baker.c:0388: unitialized baker
10:04:27.376 T0 E            baker.c:0388: unitialized baker
python3.10: /home/mark/git/d/datoviz-distributed/src/scene/viewset.c:232: dvz_view_add: Assertion `((transform) != ((void *)0))' failed.
Aborted (core dumped)

After some "scrolling around" on this repo:

11:16 $ ipython
Python 3.9.16
Type 'copyright', 'credits' or 'license' for more information
IPython 8.14.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import numpy as np
   ...: from datoviz import canvas, run, colormap
   ...: 
   ...: panel = canvas(show_fps=True).scene().panel()
   ...: visual = panel.visual('marker')
   ...: 
   ...: N = 10_000
   ...: pos = np.random.randn(N, 3)
   ...: ms = np.random.uniform(low=2, high=35, size=N)
   ...: color = colormap(np.random.rand(N), cmap='viridis')
   ...: 
   ...: visual.data('pos', pos)
   ...: visual.data('ms', ms)
   ...: visual.data('color', color)
   ...: 
   ...: run()

Installed datoviz event loop hook.

In [2]: 

In [2]: python3.9: /home/conda/staged-recipes/build_artifacts/datoviz_1687837310456/work/src/vklite.c:3224: dvz_cmd_begin_renderpass: Assertion `(framebuffers->framebuffers[iclip] != ((void*)0))' failed.

vulkaninfo attached vulkaninfo.txt

[hmaarrfk]

For the error on this repo (main/master branch), sometimes i don't even have to scroll around and it just crashes what appears to be "immediately".

Putting it in a non-interactive script,

$ python t.py 
python: /home/conda/staged-recipes/build_artifacts/datoviz_1687837310456/work/src/vklite.c:3224: dvz_cmd_begin_renderpass: Assertion `(framebuffers->framebuffers[iclip] != ((void*)0))' failed.
Aborted (core dumped)

seems to fail (near) immediately too.

[rossant]

consider the distributed branch as a scratchpad for me that is currently not usable by anyone else.. (it could actually be a private repo)

for the main branch, are the standalone C examples working?

[hmaarrfk]

consider the distributed branch as a scratchpad for me that is currently not usable by anyone else.. (it could actually be a private repo)

Of course. I was mostly testing to see if some of them were resolved.

for the main branch, are the standalone C examples working?

Not sure I installed them as part of my "conda package". I had to make some config changes to the build process too. I'll make a "PR" so that it can be clear what I changed, but it was mostly to help it find "system" libraries instead of "rebuilding" them from scratch.

I'll get back to you in a bit

[hmaarrfk]

Starting from:

$ git log -n 1
commit 2b8bdd0f03176a4b1f72c43e9c4d777fe102a77e (HEAD -> main, upstream/main, origin/main, origin/HEAD)
Merge: 097562c0 655ed09e
Author: Cyrille Rossant <[email protected]>
Date:   Mon Jun 26 10:25:30 2023 +0200

    Merge pull request #57 from hmaarrfk/library_headers
    
    Install all the headers and allow library installation

1. Patch up some build processes to use the conda-forge packages:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 07f3fac8..474ec0ed 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -32,8 +32,8 @@ set(DATOVIZ_VERSION 0.1.0)
 project(datoviz VERSION ${DATOVIZ_VERSION} DESCRIPTION "datoviz")
 
 # DEBUG/RELEASE
-set(DEBUG 1)
-set(CMAKE_BUILD_TYPE Debug)
+# set(DEBUG 1)
+# set(CMAKE_BUILD_TYPE Debug)
 
 
 # -------------------------------------------------------------------------------------------------
@@ -41,25 +41,8 @@ set(CMAKE_BUILD_TYPE Debug)
 # -------------------------------------------------------------------------------------------------
 
 # cglm
-FetchContent_Declare(
-    cglm
-    GIT_REPOSITORY  https://github.com/recp/cglm/
-    # GIT_TAG         v0.8.3
-)
-FetchContent_MakeAvailable(cglm)
-
-# glfw3
-#set(GLFW_LIBRARY_TYPE SHARED CACHE BOOL "" FORCE)
-set(GLFW_BUILD_DOCS OFF CACHE BOOL "" FORCE)
-set(GLFW_BUILD_TESTS OFF CACHE BOOL "" FORCE)
-set(GLFW_BUILD_EXAMPLES OFF CACHE BOOL "" FORCE)
-FetchContent_Declare(
-    glfw
-    GIT_REPOSITORY  https://github.com/glfw/glfw/
-    # GIT_TAG         3.3.7
-)
-FetchContent_MakeAvailable(glfw)
-
+find_package(cglm REQUIRED)
+find_package(glfw3 REQUIRED)
 # Vulkan
 if (DATOVIZ_WITH_VULKAN_SDK)
     find_package(Vulkan)
diff --git a/examples/standalone/build.sh b/examples/standalone/build.sh
index 319d90b4..0669c3fc 100755
--- a/examples/standalone/build.sh
+++ b/examples/standalone/build.sh
@@ -1,6 +1,6 @@
 # This command requires glfw with include files and libraries.
 
-if [ -z "$1" ]; then 
+if [ -z "$1" ]; then
 export DVZ_EXAMPLE_FILE="standalone_canvas.c";
 else
 export DVZ_EXAMPLE_FILE=$1
@@ -8,7 +8,7 @@ fi
 
 # This build script should be improved, use cmake perhaps
 #export DVZ_EXAMPLE_FILE=$1
-export DVZ_ROOT=../../
+# export DVZ_ROOT=../../
 export AUTOMATED=""
 if [ ! -z "$2" ]
 then
@@ -24,6 +24,8 @@ fi
 # Compile the example.
 # NOTE: use -lgfw3 on macOS
 gcc $DVZ_EXAMPLE_FILE \
+    ${CFLAGS} \
+    ${LDFLAGS} \
     $AUTOMATED \
     -I$DVZ_ROOT/include/ \
     -I$DVZ_ROOT/build/_deps/cglm-src/include \

2. Create a build.sh script to make things easier:

set -ex
mkdir -p build
pushd build

export PREFIX=${PREFIX:-${CONDA_PREFIX}}

cmake ${CMAKE_ARGS} \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=${PREFIX} \
    -DPOSITION_INDEPENDENT_CODE=ON \
    -DDATOVIZ_WITH_CLI=OFF \
    ..

make   # -j${CPU_COUNT}
make install
popd

pushd examples/standalone
bash build.sh
popd


python utils/generate_cython.py
pushd bindings/cython
python -m pip install --no-deps --no-build-isolation .
popd

3. Create your conda environment

mamba create --name datoviz --yes \
    --channel mark.harfouche --channel conda-forge --override-channels \
    compilers python=3.10 ipython \
    mesa-libgl-devel-cos7-x86_64 \
    cython numpy \
    shaderc glslang glfw cglm spirv-tools \
    libvulkan-headers libvulkan-loader \
    imageio colorcet \
    tqdm pyparsing

PS. I just merged https://github.com/conda-forge/staged-recipes/pull/23172 so the shaderc package should be available directly from conda-forge

4. Build

bash build.sh  2>&1 | tee log.txt

5. But example fails to launch with:

$ (cd examples/standalone/ && ./datoviz_example)
05:41:48.168 W           vklite.c:0655: suboptimal frame, recreate swapchain
05:41:48.172 W           vklite.c:0655: suboptimal frame, recreate swapchain
05:41:48.176 W           vklite.c:0655: suboptimal frame, recreate swapchain
05:41:48.180 W           vklite.c:0655: suboptimal frame, recreate swapchain
05:41:48.184 W           vklite.c:0655: suboptimal frame, recreate swapchain
05:41:48.188 W           vklite.c:0655: suboptimal frame, recreate swapchain
05:41:48.191 W           vklite.c:0655: suboptimal frame, recreate swapchain
05:41:48.194 W           vklite.c:0655: suboptimal frame, recreate swapchain

warnings are generated when resizing the window

Question: is there an other issue you would like me to test out?

I can't seem to recreate the crash now but i'll try again in a bit. Maybe it was due to building the package in docker vs within the same environment?

Secondary issue

Trying to enable the CLI, fails with an error that looks like:

[100%] Linking C executable datoviz
/home/mark/mambaforge/envs/datoviz/bin/../lib/gcc/x86_64-conda-linux-gnu/11.4.0/../../../../x86_64-conda-linux-gnu/bin/ld: /home/mark/mambaforge/envs/datoviz/bin/../x86_64-conda-linux-gnu/sysroot/usr/lib/../lib/gcrt1.o: relocation R_X86_64_32S against symbol `__libc_csu_fini' can not be used when making a PIE object; recompile with -fPIE
/home/mark/mambaforge/envs/datoviz/bin/../lib/gcc/x86_64-conda-linux-gnu/11.4.0/../../../../x86_64-conda-linux-gnu/bin/ld: failed to set dynamic section sizes: bad value

log.txt

[hmaarrfk]

hmm, the crashes may be a wayland thing.

On wayland, the c example above runs with the same warning:

18:35:27.426 W           vklite.c:0655: suboptimal frame, recreate swapchain

The python code crashes