InsightSoftwareConsortium / InsightSoftwareConsortium/ITK

ENH: Master tracking — Eigen3 third-party design for ITK 6.x

Open
#6,230 20 comments 2 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
1.7k
Forks
748
Avg merge
1d 1h
Merged PRs (30d)
64

Description

# ENH: Master tracking — Eigen3 third-party design for ITK 6.x

## Why this issue

Thanks to @blowekamp and @dzenanz for suggesting that the Eigen design conversation needs a single venue rather than continued incremental patches.

Eigen-related work has accumulated multiple half-finished refactors (PR #5952, PR #6225), recurring user-confusion threads, and explicit "let's have a design discussion before more incremental patches" requests from @blowekamp (#6225) and @dzenanz (#5952). Several maintainer positions have been stated in scattered places (Discourse 2018-2023, individual PR threads) but never consolidated.

This issue is a single venue to converge on **what we want Eigen to look like in ITK 6 and 7**, what is grandfathered from earlier decisions, and what concrete sub-PRs the consolidated direction implies. ITKv6 is permitted to break or revise historical promises where the cost-benefit warrants it; the goal here is to make those revisions intentional, not accidental.

## The historical baseline (load-bearing context)

These decisions were made deliberately and have shaped the current code:

1. **Internal Eigen3 was added as a ThirdParty module in 2018** ([Discourse #1429](https://discourse.itk.org/t/add-internal-third-party-module-eigen3/1429), authored by @phcerdan). **Stated rationale:** allow remote modules that already use Eigen (e.g. `ITKTotalVariation` bridging to `proxTV`) to share ITK's Eigen via `find_package(Eigen3)` pointed at ITK's tree. The long-term plan articulated by @matt.mccormick and @dzenanz: gradually migrate ITK internals from VNL to Eigen.

2. **Symbols are NOT mangled — by design.** @phcerdan, [Discourse #5562 (2022)](https://discourse.itk.org/t/name-mangling-for-the-eigen-library/5562): *"It doesn't [mangle]. Eigen is a header-only library and it wasn't mangled to facilitate bridging with external libraries using native Eigen."* Re-raising mangling in 2026 is fine, but must engage with this prior decision.

3. **Three documented user-facing usage modes** (@phcerdan, [Discourse #5627 (2023)](https://discourse.itk.org/t/using-itk-eigen/5627)):
- **A.** Use ITK's internal Eigen via ``
- **B.** `-DITK_USE_SYSTEM_EIGEN:BOOL=ON` + `-DEigen3_DIR:PATH=...` then ``
- **C.** Reuse ITK's internal Eigen as your project's Eigen (because no mangling)

4. **VNL → Eigen migration never materialized** beyond the 2018 plan. The aspiration is documented; the work is not staged. ITKv6 is the moment to either revive or formally retire that goal.

## Where the current friction lives

Active / open PRs

| PR | Author | State | Intent | Key blockers |
|----|--------|-------|--------|--------------|
| **#6225** | @hjmjohnson | open, draft | Migrate include sites to canonical `` via `Eigen3::Eigen` (supersedes #5952) | @blowekamp wants design discussion first; @seanm reports external-build regression; Greptile P1 on missing warning suppression under `ITK_USE_SYSTEM_EIGEN=ON` |
| **#5952** | @blowekamp | stale (Mar 2026) | `itk_Eigenvalues.h` per-module wrappers (alternative direction) | @dzenanz: only 1/2 callers migrated, contradictory CMake change; @hjmjohnson asked for closure |
| **#6224** | — | open issue | `ITK_EIGEN(Header)` macro can't carry in-source `SYSTEM` include paths | Blocks the cleanest fix |
| **#5484** | — | open issue | Eigen `-Wshadow` warnings | Fix landed upstream in Eigen MR `!1936`; awaits next vendored-Eigen update |

Recently merged (last 12 months)

- **#6176** (@hjmjohnson, merged 2026-05-06) — Vendored Eigen 5.0.1+master. Acceptance matrix vs system 3.3.9 / 3.4.0 / 5.0.1.
- **#6223** (@hjmjohnson, merged 2026-05-06) — Exposed `itkeigen` subdir for external consumers (this is what @seanm's #6225 regression report hangs off of).
- **#5831** (@blowekamp, merged 2026-03) — Large CMake refactor. @phcerdan in-thread: endorses retreating from "modules use raw Eigen", urges symbol mangling (not done).
- **#5590** (@cho-m, merged 2025-11) — System Eigen ≥ 5 support.

Recurring user-pain issues

#4407, #4840, #4006, #1888 — `ITKInternalEigen3` install brittleness across CMake versions; mostly fixed in 5.4. Worth checking that ITK 6 doesn't regress these.

## The questions that need answers before more patches land

Each is binary or short-list; the goal is to get a maintainer-quorum answer to each, then translate them into work.

### Q1. Header-path policy (the active flashpoint)

Three coexisting styles in-tree today:

```cpp
ITK_EIGEN(Eigenvalues) // legacy macro →
#include // canonical, what PR #6225 wants
#include "itk_Eigenvalues.h" // per-module wrapper, what PR #5952 wanted
```

**Decision needed:** Pick one canonical style for ITK 6 internals. The other two can remain as transitional aliases or be deprecated. Which?

### Q2. Symbol mangling — revisit or honor the 2022 decision?

@phcerdan's 2022 reason for not mangling (bridge use case C above) is real but narrow. @phcerdan also endorsed mangling on PR #5831 in 2026. Two options:

- **Honor 2022:** keep unmangled, document that downstream modules can share ITK's Eigen, accept the ABI/ODR risk when an external library also vendors Eigen.
- **Mangle in ITK 6:** explicitly drop usage mode C, gain ODR safety à la VTK. Costs: bridging modules need updates; vendored-Eigen update mechanics get more involved.

### Q3. `Eigen3::Eigen` CMake target — exposed to consumers?

Already partially the case (#6223). Decision needed: is `Eigen3::Eigen` (or `ITK::Eigen3` aliased) a **stable, supported** CMake target that downstream `find_package(ITK)` consumers can link against, or an internal detail that may be removed?

### Q4. Warning suppression scope

When `ITK_USE_SYSTEM_EIGEN=ON`, current code does not pass the same `-Wno-*` flags that the internal Eigen build uses. Greptile's P1 on #6225 surfaces this. Two options:

- Treat system Eigen as "user's responsibility, we don't suppress."
- Apply the same suppressions for both — uniform but means leaking flags into consumer builds.

### Q5. VNL → Eigen migration — revive or formally retire?

The 2018 aspiration is unimplemented. Either:

- **Revive:** stage it for ITK 7+, accept it requires a multi-release deprecation cycle (`GetVnlVector` → `GetInternalVector` etc., per @phcerdan 2018).
- **Retire:** explicitly document VNL as the canonical numerics layer; relegate Eigen to its current niches (registration metrics, certain filters, third-party bridges) and remove "VNL replacement" from the roadmap.

### Q6. Where does the design discussion happen?

GitHub issue (this one), Discourse thread, or a community-call agenda item? @blowekamp asked for "a discussion" on #6225 — naming the venue avoids the discussion happening in 5 places at once.

### Q7. ITKv6 license to break

ITKv6 may revise historical promises where they no longer pay rent. Each Q above should be answered for ITK 6, **not** "what would have been compatible with the 2018 design." If the 2018 bridge-use case has zero real users in 2026, the design moves; if it has real users, accommodation matters.

## Maintainer positions summarized (best understanding)

| Maintainer | Position |
|------------|----------|
| @phcerdan | Original architect; bus-factor on vendored-update dance; OK to abandon "raw-Eigen-in-modules"; urged mangling on PR #5831 |
| @blowekamp | Wants top-level design discussion before more patches; flagged "Eigen usage is rough around the edges" |
| @dzenanz | Permissive reviewer; "Eigen is extra complicated"; original VNL-replacement champion |
| @thewtex | 2018: explicit Eigen-as-VNL-successor goal |
| @N-Dekker | Prefers tag-pinned over master-tip vendoring (raised on #4265, never addressed) |
| @seanm | Pushes fixes upstream; weak on subtree mechanics |
| @hjmjohnson | Filed this issue; current proposer of #6225 canonical-headers approach |

## Concrete asks of the maintainer group

1. Each maintainer (especially the seven listed above) leaves a comment with their position on **Q1–Q5**. Even one-line "I prefer option B because X" is enough.
2. Once Q1 has a quorum answer, **#6225 either lands as the canonical-headers patch OR is closed in favor of #5952's per-module-wrapper direction.** No more half-merged refactors.
3. If Q5 says "revive," a separate ENH umbrella issue is opened to stage the VNL → Eigen migration for ITKv7.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.