Add Julia support to gprc4bmi #144

sverhoeven · 2023-09-29T14:27:31Z

grpc4bmi/bmi_julia_model.py

To resolve: warning The version of Java (11.0.3) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.

This reverts commit 4f1de4e.

test/test_julia.py

BSchilperoort · 2023-10-17T11:53:57Z

grpc4bmi/bmi_julia_model.py

+    Args:
+        package: Name of package to install.
+    """
+    jl.Pkg.add(package)


Specific URLs and git branches can also be installed. See https://pkgdocs.julialang.org/v1/api/#Package-API-Reference

import Pkg Pkg.add(name="Wflow", rev="BMI")

or

Pkg.add(url="https://github.com/JuliaLang/Example.jl", rev="master")

Removed install method and use jl.Pkg.add() direct;ly.

sverhoeven · 2023-10-17T13:05:42Z

Calling a bmi function in julia via a grpc python server from a grpc client causes the process to use 100% cpu and not return a response.

Seems grpc and juliacall can not live together.

…m/csdms/bmi-example-julia methods

Need to add handle_julia_exception from https://blog.esciencecenter.nl/10-examples-of-embedding-julia-in-c-c-66282477e62c to get better error Got error: 1.4142135623730951 [7] signal (11.1): Segmentation fault in expression starting at none:0 jl_lookup_generic_ at /cache/build/default-amdci5-5/julialang/julia-release-1-dot-9/src/gf.c:2819 [inlined] ijl_apply_generic at /cache/build/default-amdci5-5/julialang/julia-release-1-dot-9/src/gf.c:2936 jl_apply at /cache/build/default-amdci5-5/julialang/julia-release-1-dot-9/src/julia.h:1880 [inlined] ijl_call1 at /cache/build/default-amdci5-5/julialang/julia-release-1-dot-9/src/jlapi.c:230 main at /usr/local/bin/run_bmi_server (unknown line) __libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line) _start at /usr/local/bin/run_bmi_server (unknown line) Allocations: 2060272 (Pool: 2059240; Big: 1032); GC: 3 Segmentation fault

sverhoeven · 2023-10-17T14:37:42Z

Now trying to write c wrapper around julia in https://github.com/eWaterCycle/grpc4bmi/tree/julia/test/heat-images/c-julia

review-notebook-app · 2023-10-18T09:34:24Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

sverhoeven · 2023-10-18T09:45:25Z

Got it to work until it segfaults somewhere as seen in https://github.com/eWaterCycle/grpc4bmi/blob/julia/test/heat-images/c-julia/demo.ipynb. With following server output:

docker build -t heat:c-julia -f test/heat-images/c-julia/Dockerfile .
docker run -ti --rm -p 55555:55555 heat:c-julia
BMI grpc server attached to server address 0.0.0.0:55555
GetComponentName1
Heat.Model at time = 0.0 and mean temperature = 0.43864346382398506
BMI.get_component_name(model)
GetComponentName1

[1] signal (11.1): Segmentation fault
in expression starting at none:0
Allocations: 2058140 (Pool: 2057109; Big: 1031); GC: 3

I think this has to do with grpc creating threads for each incoming request. If BMI.initialize was not called in current thread then segfault happens.

I tried to limit max threads to 2, but segfaults still happen, but less frequently.

https://scientificcoder.com/extreme-multi-threading-c-and-julia-19-integration looks like what I need to use.

I do not know how to make it run without segfaulting, maybe @abelsiqueira or @goord know a way?

abelsiqueira · 2023-10-19T13:23:15Z

I can reproduce the error :)

I tried some things for a couple of hours, but I've made no progress. Using mtx.lock() and unlock is not enough. Using jl_adopt_thread() creates a different segmentation fault at initialization:

[1] signal (6.-6): Aborted
in expression starting at none:0
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
jl_init_threadtls at /cache/build/default-amdci5-5/julialang/julia-release-1-dot-9/src/threading.c:333
ijl_adopt_thread at /cache/build/default-amdci5-5/julialang/julia-release-1-dot-9/src/threading.c:413
_ZN8BmiJulia10InitializeENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE at /usr/local/bin/run_bmi_server (unknown line)
...

which means nothing to me.

The other functions jl_gc_safe_enter and jl_enter_threaded_region were not found by the compiler. Haven't checked why.

Then I tried PackageCompiler, but you need to define the functions as ccallable, and that requires changing some code from bmi-example or something, and I don't have experience doing that. The most basic test failed because the type Model was not concrete (although I think it is).

Overall, I don't have enough knowledge of the project, nor of C++, nor of Julia+C++ integration, nor threads, so at this point I am just randomly testing things.

Let me know if you need a longer consultation to try to make this work, and sorry for not being more helpful.

sverhoeven · 2023-10-20T07:41:01Z

I can reproduce the error :)

I tried some things for a couple of hours, but I've made no progress. Using mtx.lock() and unlock is not enough. Using jl_adopt_thread() creates a different segmentation fault at initialization:
[1] signal (6.-6): Aborted
in expression starting at none:0
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
jl_init_threadtls at /cache/build/default-amdci5-5/julialang/julia-release-1-dot-9/src/threading.c:333
ijl_adopt_thread at /cache/build/default-amdci5-5/julialang/julia-release-1-dot-9/src/threading.c:413
_ZN8BmiJulia10InitializeENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE at /usr/local/bin/run_bmi_server (unknown line)
...
which means nothing to me.

The other functions jl_gc_safe_enter and jl_enter_threaded_region were not found by the compiler. Haven't checked why.

Then I tried PackageCompiler, but you need to define the functions as ccallable, and that requires changing some code from bmi-example or something, and I don't have experience doing that. The most basic test failed because the type Model was not concrete (although I think it is).

Overall, I don't have enough knowledge of the project, nor of C++, nor of Julia+C++ integration, nor threads, so at this point I am just randomly testing things.

Let me know if you need a longer consultation to try to make this work, and sorry for not being more helpful.

Thanks for looking into this. We will go for wrapping the Julia model directly in Python with juliacall without grpc in-between the Python client and model.

@BSchilperoort you already have this working in https://github.com/eWaterCycle/pywflow, but it would be nice if BmiJulia class could be used there.

BSchilperoort · 2023-10-24T14:49:57Z

I modified bmi_julia_model.py, as the numpy arrays have to be converted before passing to Julia.

sverhoeven · 2023-10-25T13:19:05Z

Hmm test gets stuck in test_subproc

sonarqubecloud · 2023-10-26T14:04:48Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

BSchilperoort

Shame that Julia doesn't play nice with gRPC 😞

The local model works well. I am able to use it for Wflow.jl, and haven't run into any issues anymore (except the init thing, see the comment).

BSchilperoort · 2023-10-27T07:51:53Z

grpc4bmi/bmi_julia_model.py

+        jl.seval("import " + package4implementation)
+        model = jl.seval(model_name)
+        implementation = jl.seval(implementation_name)
+        return BmiJulia(model, implementation)


One problem with this: you cannot add this to an eWaterCycle LocalModel. That requires a callable class, i.e. one that is not initialized yet.

For Wflow.jl I had to do the following to make it work with eWaterCycle:

class WflowBmi(BmiJulia): def __init__(self): m = self.from_name("Wflow.Model", implementation_name="Wflow.BMI") super().__init__(m.model, m.implementation)

ah, it made sense for grpc server from cli, but not as a super class.

Created #145

BSchilperoort · 2023-10-27T07:53:42Z

test/heat-images/c-julia/README.md

Seeing as it does not work, do we want to leave these files here? Do we want to attempt this again later?

I wanted to keep it because it was a lot of work, and its already located in a nook in the repo.

sverhoeven

Thanks for looking at it.

sverhoeven · 2023-10-30T10:03:02Z

grpc4bmi/bmi_julia_model.py

+        jl.seval("import " + package4implementation)
+        model = jl.seval(model_name)
+        implementation = jl.seval(implementation_name)
+        return BmiJulia(model, implementation)


ah, it made sense for grpc server from cli, but not as a super class.

Created #145

sverhoeven · 2023-10-30T10:04:39Z

test/heat-images/c-julia/README.md

I wanted to keep it because it was a lot of work, and its already located in a nook in the repo.

Add Julia support to gprc4bmi

b913f8d

Refs #101

BSchilperoort reviewed Oct 2, 2023

View reviewed changes

grpc4bmi/bmi_julia_model.py Outdated Show resolved Hide resolved

BSchilperoort reviewed Oct 2, 2023

View reviewed changes

grpc4bmi/bmi_julia_model.py Outdated Show resolved Hide resolved

BSchilperoort reviewed Oct 2, 2023

View reviewed changes

grpc4bmi/bmi_julia_model.py Outdated Show resolved Hide resolved

Switch to 2 arg constructor + Add tests for julia

57bb251

BSchilperoort mentioned this pull request Oct 2, 2023

Add wflow julia eWaterCycle/ewatercycle#374

Open

sverhoeven and others added 9 commits October 2, 2023 16:02

Add output arg + get package from model name + more tests

3607d56

Convert Julia type to Python type

2b612dc

Force install of heat.jl

5d64b15

Make sure BasicModelInterface.jl is installed

45c9879

Upgrade sonar action

6bb4e47

To resolve: warning The version of Java (11.0.3) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.

Dont need BasicModelInterface.jl to be installed explicitly?

4f1de4e

Revert "Dont need BasicModelInterface.jl to be installed explicitly?"

340067e

This reverts commit 4f1de4e.

Try to test against a jl file

ceff03c

Try to test against a jl file (python side)

ac4fdac

BSchilperoort reviewed Oct 16, 2023

View reviewed changes

test/test_julia.py Outdated Show resolved Hide resolved

sverhoeven and others added 3 commits October 16, 2023 16:00

Fake model.jl does not config file

d5d38cd

More tests + py index to julia index + 1

badc5f7

Fix get_value_at_indices

a8d4e59

BSchilperoort reviewed Oct 17, 2023

View reviewed changes

Make implementatation name settable from cli

daeca16

sverhoeven added 3 commits October 17, 2023 15:36

Trying to call Julia from C

575b0f9

Calling sqrt in julia from c works, now try to call https://github.co…

fe42fc7

…m/csdms/bmi-example-julia methods

Got it to work, until it segfaults somewhere

05e1b47

TODOs

ffbe702

Convert numpy array to Julia vector

0a2037a

sverhoeven added 5 commits October 25, 2023 11:16

Remove install method use jl.Pkg.add directly

9f413af

Keep BmiJulia and heat-images/julia-c, remove other mentions of Julia

125560a

Newline

7d1c3e5

Add note that test/heat-images/c-julia is broken

a54c731

Add timeout to test

c6bddde

sverhoeven and others added 4 commits October 26, 2023 08:01

Log thread id

ead67cd

Merge branch 'julia' of github.com:eWaterCycle/grpc4bmi into julia

4333f13

Increase delay in BmiClientSubProcess tests

7c29974

Dont wait forever after subproces is terminated

3fbb905

sverhoeven marked this pull request as ready for review October 26, 2023 12:22

sverhoeven requested a review from BSchilperoort October 26, 2023 12:22

Kill subprocesses instead of letting them terminate

2fb9ffb

BSchilperoort approved these changes Oct 27, 2023

View reviewed changes

sverhoeven commented Oct 30, 2023

View reviewed changes

sverhoeven merged commit 37040d6 into main Oct 30, 2023
3 checks passed

sverhoeven deleted the julia branch October 30, 2023 10:05

sverhoeven mentioned this pull request Nov 13, 2023

OpenAPI support #146

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Julia support to gprc4bmi #144

Add Julia support to gprc4bmi #144

sverhoeven commented Sep 29, 2023

BSchilperoort Oct 17, 2023

sverhoeven Oct 25, 2023

sverhoeven commented Oct 17, 2023

sverhoeven commented Oct 17, 2023

review-notebook-app bot commented Oct 18, 2023

sverhoeven commented Oct 18, 2023

abelsiqueira commented Oct 19, 2023

sverhoeven commented Oct 20, 2023

BSchilperoort commented Oct 24, 2023

sverhoeven commented Oct 25, 2023

sonarqubecloud bot commented Oct 26, 2023

BSchilperoort left a comment

BSchilperoort Oct 27, 2023

sverhoeven Oct 30, 2023

BSchilperoort Oct 27, 2023

sverhoeven Oct 30, 2023

sverhoeven left a comment

sverhoeven Oct 30, 2023

sverhoeven Oct 30, 2023

Add Julia support to gprc4bmi #144

Add Julia support to gprc4bmi #144

Conversation

sverhoeven commented Sep 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sverhoeven commented Oct 17, 2023

sverhoeven commented Oct 17, 2023

review-notebook-app bot commented Oct 18, 2023

sverhoeven commented Oct 18, 2023

abelsiqueira commented Oct 19, 2023

sverhoeven commented Oct 20, 2023

BSchilperoort commented Oct 24, 2023

sverhoeven commented Oct 25, 2023

sonarqubecloud bot commented Oct 26, 2023

BSchilperoort left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sverhoeven left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment