Skip to content

Update G4Vis to breaking changes in GeometryBasics@0.5#27

Merged
peremato merged 3 commits intoJuliaHEP:masterfrom
fhagemann:g4vis
Jun 11, 2025
Merged

Update G4Vis to breaking changes in GeometryBasics@0.5#27
peremato merged 3 commits intoJuliaHEP:masterfrom
fhagemann:g4vis

Conversation

@fhagemann
Copy link
Contributor

@fhagemann fhagemann commented Jun 6, 2025

According to the release notes of GeometryBasics@0.5.0, there seem to be some breaking changes that affect the G4Vis extension, and also currently result in the documentation CI failing.

These changes are:

  • Tesselation was renamed to Tessellation
  • meta() removed: Mesh(meta(kwargs...), faces) -> Mesh(faces; kwargs...)
  • mesh(..., normals = ...) -> mesh(..., normal = ...)

Also, looks like CairoMakie.draw_mesh3D requires the normals to be of type Union{Nothing, Vec3f} (so in this case Vec3f = Vec3{Float32}), but G4Vis by default creates normals of type Vec3{Float64} and struggles converting it because of the Union. Therefore, I explicitly require the normaltype to be Vec3f.

I also bumped the minimal compat for GeometryBasics to 0.5. As far as I know, GeometryBasics is only relevant in the G4Vis extension.

Let's see whether the docs CI runs without failing now 🤞

@codecov
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Project coverage is 1.76%. Comparing base (6d5d520) to head (17b9c6a).
Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
ext/G4Vis/Boolean.jl 0.00% 6 Missing ⚠️
ext/G4Vis/G4Vis.jl 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           master     #27      +/-   ##
=========================================
- Coverage    1.78%   1.76%   -0.03%     
=========================================
  Files          17      17              
  Lines        1231    1250      +19     
=========================================
  Hits           22      22              
- Misses       1209    1228      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Moelf
Copy link
Member

Moelf commented Jun 6, 2025

@fhagemann
Copy link
Contributor Author

I will look at this specific example locally and see whether I can learn anything from it.. -.-

@fhagemann
Copy link
Contributor Author

This specific example runs locally...
image

@Moelf
Copy link
Member

Moelf commented Jun 6, 2025

does the Project.toml in docs/ need to have compat? This is why having two Project.toml is annoying I guess

@fhagemann
Copy link
Contributor Author

fhagemann commented Jun 6, 2025

I don't have compat entries in the docs/Project.toml locally. I could try adding one though

@peremato
Copy link
Member

peremato commented Jun 6, 2025

This is why having two Project.toml is annoying I guess

Because it has additional dependencies (e.g. Literate, PYTHIA8, etc.)

@peremato
Copy link
Member

peremato commented Jun 6, 2025

https://github.com/JuliaHEP/Geant4.jl/actions/runs/15480067026/job/43584141833?pr=27#step:9:59

damn why is this so unhelpful

Because to generate the documentation (create and run the notebooks) we need to run Geant4 several times and this can only be done in separate processes since the cleanup of Geant4 C++ is not perfect (many singletons left behind).

@peremato
Copy link
Member

peremato commented Jun 6, 2025

I am can build successfully the documentation locally (MacOS) but fails in the CI with signal 11 (segmentation fault). I'll try in a Linux system. I suspect the IGLWrap_jll binary library that is used to draw meshes of boolean solids, which is not used on MacOS.

@peremato
Copy link
Member

peremato commented Jun 6, 2025

@fhagemann Yes the problem is that GeometryBasics has changed the internal layout and the interface to IGLWrap does not work anymore. For the time being to get this PR we can disable drawing boolean solids with something like this:

--- a/ext/G4Vis/Boolean.jl
+++ b/ext/G4Vis/Boolean.jl
@@ -40,11 +40,12 @@ function boolean(op, m1::GeometryBasics.Mesh, m2::GeometryBasics.Mesh, t::Transf
         ncoors(cm)::Ptr{Cint}, nfaces(cm)::Ptr{Cint},
         vpointer(cm)::Ptr{Ptr{Cdouble}}, fpointer(cm)::Ptr{Ptr{Cint}},
         j::Ref{Ptr{Cint}})::Cint
+    @show ret
 
     if ret == 0
         c = unsafe_array(Point3{Float64}, cm.vertices, cm.nv)
         f = unsafe_array(TriangleFace{Int32}, cm.faces, cm.nf)
-        return GeometryBasics.Mesh(c,f)
+        return GeometryBasics.mesh(c,f)
     else
         return
     end
@@ -62,13 +63,15 @@ const operation = Dict("G4UnionSolid" => 0, "G4IntersectionSolid" => 1, "G4Subtr
 
 function GeometryBasics.mesh(s::G4BooleanSolid)
     if  isdefined(IGLWrap_jll, :libiglwrap)
-        op = operation[GetEntityType(s)]
-        left = GetConstituentSolid(s, 0)
-        right = GetConstituentSolid(s, 1)
-        boolean(op, GeometryBasics.mesh(left), GeometryBasics.mesh(right))
+        println("IGLWrap_jll is available but interface needs to be fixed for drawing boolean solids (TODO)")
+        GeometryBasics.mesh(Point3{Float64}[], QuadFace{Int32}[])
+        #op = operation[GetEntityType(s)]
+        #left = GetConstituentSolid(s, 0)
+        #right = GetConstituentSolid(s, 1)
+        #boolean(op, GeometryBasics.mesh(left), GeometryBasics.mesh(right))
     else
-        println("IGLWrap_jll is not available for currrent platform $(Sys.MACHINE) and is needed for drawing boolean solids")
-        GeometryBasics.Mesh(Point3{Float64}[], QuadFace{Int32}[])
+        println("IGLWrap_jll is not available for current platform $(Sys.MACHINE) and is needed for drawing boolean solids")
+        GeometryBasics.mesh(Point3{Float64}[], QuadFace{Int32}[])
     end
 end
 
@@ -80,7 +83,7 @@ function GeometryBasics.mesh(s::G4DisplacedSolid)
     points = GeometryBasics.coordinates(m)
     faces  = GeometryBasics.faces(m)
     map!(c -> c * t, points, points)
-    GeometryBasics.Mesh(points, faces)
+    GeometryBasics.mesh(points, faces)
 end
 
 function Geant4.draw(solid::G4BooleanSolid; wireframe::Bool=false, kwargs...)

@Moelf
Copy link
Member

Moelf commented Jun 6, 2025

Because it has additional dependencies (e.g. Literate, PYTHIA8, etc.)

there's still way to do it, we can use [extras] and then [targets] like:

test = ["Test", "Pkg"]

a target of test corresponds to /test and we just need a docs target

@fhagemann
Copy link
Contributor Author

I h

Because it has additional dependencies (e.g. Literate, PYTHIA8, etc.)

there's still way to do it, we can use [extras] and then [targets] like:

test = ["Test", "Pkg"]

a target of test corresponds to /test and we just need a docs target

I had a quick look at this: this might require some work because the docs build activates the environment in the docs directory (const project = @__DIR__ ):

Geant4.jl/docs/make.jl

Lines 1 to 13 in c715fa4

using Documenter, Literate, Geant4
const examplesdir = joinpath(@__DIR__, "src/examples")
const project = @__DIR__
function process_literate(names...)
examples_mds = []
for name in names
run(`julia --project=$project docs/literate.jl $name`)
push!(examples_mds, "examples/$name.md")
end
return examples_mds
end

If I "just" delete the docs/Project.toml and add all information via [extras] and [targets] to the main Project.toml, the docs build fails immediately.

@Moelf
Copy link
Member

Moelf commented Jun 11, 2025

run(`julia --project=$project docs/literate.jl $name`)

my understanding is this shouldn't be done manually, because the doc-building GitHub action should activate docs environment as defined in the top-level Project.toml? I haven't tried

@peremato
Copy link
Member

As I already said, each notebook needs to be created in a new process (invoking Julia each time) because of Geant4 not cleaning everything after each run.

@fhagemann
Copy link
Contributor Author

Is this good to be merged?
And will anybody find time to look at how to properly mesh boolean solids?

@peremato
Copy link
Member

And will anybody find time to look at how to properly mesh boolean solids?

I'll have a look myself. I feel responsible of having done it this way.

@peremato peremato merged commit e1eba4b into JuliaHEP:master Jun 11, 2025
4 of 6 checks passed
@fhagemann fhagemann deleted the g4vis branch June 13, 2025 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants