Skip to content

feat: add support for python geometry plugins#1626

Open
wdconinc wants to merge 4 commits into
AIDASoft:masterfrom
wdconinc:python-geo-plugin
Open

feat: add support for python geometry plugins#1626
wdconinc wants to merge 4 commits into
AIDASoft:masterfrom
wdconinc:python-geo-plugin

Conversation

@wdconinc

@wdconinc wdconinc commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

This PR adds a DECLARE_DETELEMENT plugin DD4hep_PythonDetector that delegates geometry construction to a Python function specified by module/function/pythonpath XML attributes. This enables geometry to be distributed as pure Python packages without any per-project C++ compilation.

This PR also adds DDCore/python/dd4hep_geo_plugin.py providing the cppyy workarounds and type aliases needed by plugin authors (to be imported with from dd4hep_geo_plugin import *).

This PR also ports the IronCylinder example (the simplest I could find) from a C++ example to a minimal Python example, and includes it as a test.

BEGINRELEASENOTES

  • feat: add support for python geometry plugins

ENDRELEASENOTES

Note: This is for discussion; as I understand it from discussion with @jmcarcell there may be more computational intensive stuff done in the geometry plugins in FCC so this may impact initialization time in production and may add a translation step in project transition from prototype to production. In the case of EIC we have a challenge where incoming workforce is generally more familiar with python and this address that.

…lpers

Adds a DECLARE_DETELEMENT plugin 'DD4hep_PythonDetector' that delegates
geometry construction to a Python function specified by module/function/
pythonpath XML attributes. This enables geometry packages to be distributed
as pure Python without any per-project C++ compilation.

Also adds DDCore/python/dd4hep_geo_plugin.py providing the cppyy workarounds
and type aliases needed by plugin authors (from dd4hep_geo_plugin import *).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Apr 30, 2026

Copy link
Copy Markdown

Test Results

   18 files     18 suites   6h 16m 29s ⏱️
  358 tests   358 ✅ 0 💤 0 ❌
3 152 runs  3 152 ✅ 0 💤 0 ❌

Results for commit 67cf7c3.

♻️ This comment has been updated with latest results.

@wdconinc wdconinc force-pushed the python-geo-plugin branch from c84247b to 8828eb6 Compare April 30, 2026 15:10
@wdconinc

Copy link
Copy Markdown
Contributor Author

Aha, interesting, https://github.com/wdconinc/DD4hep/actions/runs/25173235648/job/73798348230 fails since this requires ROOT v6.34 for TPython::SwapWithObjAtAddr. I'll make this functionality conditional on ROOT version.

@wdconinc wdconinc marked this pull request as ready for review April 30, 2026 21:21
Copilot AI review requested due to automatic review settings April 30, 2026 21:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new detector factory that delegates DD4hep geometry construction to Python, plus a small helper module and an example/test demonstrating the workflow.

Changes:

  • Add DD4hep_PythonDetector (DECLARE_DETELEMENT) that imports a Python module/function specified in compact XML and returns a DetElement built in Python.
  • Add DDCore/python/dd4hep_geo_plugin.py helper module (cppyy workarounds + commonly used DD4hep type aliases) for plugin authors.
  • Add a Python port of the IronCylinder example + compact XML and wire it into ClientTests with a ROOT/TPython availability guard.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
examples/ClientTests/python/IronCylinder_python_geo.py New Python geometry factory function for the IronCylinder example.
examples/ClientTests/compact/IronCylinder_python.xml New compact XML using DD4hep_PythonDetector to invoke the Python geometry.
examples/ClientTests/CMakeLists.txt Install python/ example dir and add a new ClientTests regression test guarded by ROOT TPython support.
DDCore/src/python/PythonPlugin.cpp Implement and register the DD4hep_PythonDetector factory; add env expansion + Python invocation glue.
DDCore/python/dd4hep_geo_plugin.py New helper module for Python-based geometry plugins (cppyy helpers + type aliases).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +23 to +27
// C++ includes
#include <cstdlib>
#include <regex>
#include <sstream>

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

py_str() uses snprintf and create_python_detector() uses std::uintptr_t, but this file doesn't include the headers that define them. Please add the appropriate includes (e.g. <cstdio> for snprintf and <cstdint> for std::uintptr_t) to avoid non-portable builds that may fail depending on transitive includes / compiler settings.

Copilot uses AI. Check for mistakes.
result.append(input, last_pos, m.position() - last_pos);
const std::string var = m[1].matched ? m[1].str() : m[2].str();
const char* val = std::getenv(var.c_str());
if (val) result.append(val);

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expand_env() silently drops $VAR / ${VAR} tokens when the environment variable is not set, which can turn a configured pythonpath into an unintended absolute/relative path (e.g. ${DD4hepExamplesINSTALL}/... becomes /...) and make failures hard to diagnose. Consider either leaving the original token intact when getenv returns null, or throwing a DD4hep exception that clearly names the missing environment variable.

Suggested change
if (val) result.append(val);
if (!val) {
except("PythonPlugin",
"Missing environment variable '%s' while expanding: %s",
var.c_str(), input.c_str());
}
result.append(val);

Copilot uses AI. Check for mistakes.
Comment on lines +209 to +213
const std::string mod = get_attr("module");
const std::string func = get_attr("function");
const std::string pypath = has_attr("pythonpath")
? expand_env(get_attr("pythonpath")) : "";

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module and function are required attributes for this factory, but the code reads them via attr<std::string> without a prior hasAttr check. If either is missing, the thrown exception will likely come from the XML layer and may not clearly indicate what's required for DD4hep_PythonDetector. Consider explicitly validating presence and raising a factory-specific error message (including the detector name/id and the missing attribute).

Copilot uses AI. Check for mistakes.
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.

2 participants