Skip to content

Fixes for PR717#1

Draft
hughcars wants to merge 43 commits into
Pennycook:cudssfrom
awslabs:hughcars/pr717-cudss
Draft

Fixes for PR717#1
hughcars wants to merge 43 commits into
Pennycook:cudssfrom
awslabs:hughcars/pr717-cudss

Conversation

@hughcars

@hughcars hughcars commented Jun 17, 2026

Copy link
Copy Markdown

Fixes for PR717:

  • restack the patches
  • tweak the CI check

@hughcars hughcars force-pushed the hughcars/pr717-cudss branch 3 times, most recently from fe5c897 to e6f2324 Compare June 22, 2026 17:17
@hughcars hughcars changed the title Draft: rebase PR #717 cuDSS support onto current main while preserving provenance Fixes for PR717 Jun 22, 2026
Add WavePortOperator::AddBoundaryMassBdrCoefficients and GetWavePortKn
helpers, plus SpaceOperator::GetWavePortBoundaryMassMatrix, exposing
the per-port μ⁻¹ boundary mass M^(p) and the scalar kₙ,p(ω) separately.
The existing AddExtraSystemBdrCoefficients now calls the new helper
internally, so behaviour and assembled operators are unchanged.

These helpers are foundational plumbing for upcoming changes to the
RomOperator: storing per-port projected M^(p)_r once and assembling
the wave-port contribution online as Σ_p kₙ,p(ω)·M^(p)_r recovers an
O(n²) online phase for adaptive sweeps with wave ports, and unblocks
circuit synthesis with wave ports.

Adds a unit test that compares Im{A2(ω)·v} to Σ_p kₙ,p(ω)·M^(p)·v on
the existing CPW wave-port example at three frequencies in the
configured sweep band, in serial and under MPI.

(cherry picked from commit 9248d8f)
The reduced-order driven sweep currently re-assembles the full HDM
A2(ω) and projects it onto the basis at every online frequency, which
defeats the asymptotic cost model of the reduced model: each online
solve was paying for one HDM-scale boundary form assembly plus
dim_V HDM-size mat-vecs.

Wave ports are the dominant A2 contributor in practice. Their
contribution factors as A_wp(ω) = i·Σ_p kₙ,p(ω)·M^(p)_{μ⁻¹}, with
kₙ,p the only ω-dependent factor (a scalar from the per-port
cross-section EVP). Store the per-port HDM operators in RomOperator,
project them once when the basis grows, and apply the wave-port term
online as Σ_p kₙ,p(ω)·M^(p)_r. Online cost per ω drops from
HDM-scale to O(n²).

To keep correctness for any non-wave-port A2 contributors
(second-order farfield, surface conductivity), add an
include_wave_ports flag to GetExtraSystemMatrix and call the
"excluding wave ports" path on the slow fallback. The default
behaviour is unchanged for all existing call sites.

Validated against the parent branch on cpw_wave_adaptive: port-S
agrees to ~1e-11 relative, domain-E to ~1e-13.

(cherry picked from commit 9938828)
(cherry picked from commit b5cdbd08bcf90ea3ed24c7c4da6c6e4eca35fab5)
…tion

Two post-rebase cleanups:

- test-waveportoperator.cpp now loads the cpw config and mesh from
  PALACE_TEST_DATA_DIR/regression/input/cpw (staged via the regression
  fixtures that symlink to examples/cpw and are dereferenced on install)
  instead of a dedicated PALACE_SOURCE_EXAMPLES_DIR pointing at the source
  tree. Drop the now-unused PALACE_SOURCE_EXAMPLES_DIR compile definition.

- Make the port index p a consistent subscript in the comments: the
  per-port boundary mass is now written M_{μ⁻¹,p} (and M_{r,p}, M_p) to match
  kₙ,p, rather than mixing a superscript M^(p) with the subscript kₙ,p.
Put both subscripts in one group so the port index p is a true subscript
(matching M_{μ⁻¹,p}) rather than a baseline ",p" after the Unicode kₙ. Also
render the standalone constant as k_n for a consistent brace-subscript style.
Comment-only.
@hughcars hughcars force-pushed the hughcars/pr717-cudss branch 4 times, most recently from f90412f to 851d8b0 Compare July 1, 2026 17:29
simlapointe and others added 3 commits July 1, 2026 12:20
Remove unusual superbuild combinations.
@hughcars hughcars force-pushed the hughcars/pr717-cudss branch from 851d8b0 to 598ae7f Compare July 1, 2026 19:37
@hughcars hughcars force-pushed the hughcars/pr717-cudss branch 2 times, most recently from a3795eb to 60f5261 Compare July 8, 2026 19:54
Bind untrusted github.* and inputs.* context values to env variables and
reference them as shell variables in run: blocks, instead of interpolating
them directly into the script text. This closes the remaining instances of
the script-injection class flagged by AppSec (Talos) on main:

- docs.yml: "Decide whether to build" (github.ref, github.event_name,
  steps.filter.outputs.test), "Build and deploy" (inputs.deploy_tag), and
  "Verify stable docs deployment" (inputs.deploy_tag || github.ref_name)
- ghcr-cleanup.yml: both version-count steps (github.repository_owner)

Completes the hardening started in #787 (which covered build-and-test-linux,
long-tests, and long-test-status-manager). Follows GitHub's security-hardening
guidance for untrusted input.
@hughcars hughcars force-pushed the hughcars/pr717-cudss branch 2 times, most recently from 1cd11c9 to 79043ce Compare July 9, 2026 20:24
* Fix for Spack PR 52627

spack/spack#52627 changes how env is resolved.
So, here we add PWD to search for the Dockerfile template in the current
directory as well

* Pin Spack to the 1.2 release series instead of develop

Pin the Spack tooling version used by CI to the 1.2 release series
(releases/v1.2, currently 1.2.1) instead of the moving develop branch,
so CI builds are reproducible:

- setup-runner: default spack-version and all callers (palace-ci,
  build-container, run-regression-tests) now check out releases/v1.2
- container spack.yaml: images.spack pinned to the prebuilt
  spack/ubuntu-noble:1.2 Docker Hub image
- docs.yml: setup-spack@v3 gets spack_ref=releases/v1.2 and
  packages_ref=releases/v2026.06 (spack 1.2 uses the separate
  spack-packages repo); the old 'ref' key was a no-op for @V3

Binary-cache channel names (binaries.spack.io/develop and the
palace-develop-testing GHCR cache) are left unchanged: they are cache
identifiers, not tooling versions, and still resolve under Spack 1.2.

* Add spack-develop lookahead job

Expose a spack-version input on the palace-ci action (defaulting to the
pinned releases/v1.2) and add a lookahead job that builds and tests with
Spack develop. This gives an early signal when upstream Spack or
spack-packages changes break Palace, without affecting the pinned CI.
@hughcars hughcars force-pushed the hughcars/pr717-cudss branch 2 times, most recently from 5c76bcc to 5f205a9 Compare July 10, 2026 03:12
Previously, MeasureSParameter() skipped S-parameter post-processing when a
Floquet port coexisted with lumped or wave ports, but InitializePortS() did
not replicate that condition, so port-S.csv was still written with the raw
port-mode projections. The written values were wrong in two ways:

- For lumped/wave-driven excitations, the incident subtraction (-1 on the
  diagonal) was never applied, so diagonal entries were 1 + S_ii instead of
  S_ii.
- Floquet ports normalize the incident wave to unit time-averaged power
  (P_avg = 1), while lumped/wave ports normalize to unit peak power
  (integral of E x H* . n dS = 1, so P_avg = 1/2). Cross-type observations
  were therefore off by sqrt(2) in amplitude (3.01 dB in power).

Support the mixed configuration by applying a sqrt(2) power normalization
bridge to cross-type S-parameters: divide lumped/wave observations by
sqrt(2) when a Floquet port drives, and multiply Floquet observations by
sqrt(2) when a lumped/wave port drives. This restores S-matrix unitarity
(power balance) and complex reciprocity for the mixed case, verified on
Floquet + lumped and Floquet + wave port test structures (sum |S|^2 = 1 to
better than 0.01% for lossless structures, S_ij = S_ji exactly).

Floquet-only and lumped/wave-only configurations are unaffected: the bridge
factor applies only to cross-type observations.
Two new regression cases that validate the sqrt(2) power normalization
bridge for cross-type S-parameter observations:

- floquet_lumped: periodic unit cell (1x1x4 cm) with a Floquet TM port and
  a lumped resistive port (R = 377 Ohm). Verifies S-matrix unitarity
  (sum |S|^2 = 1.000) and reciprocity for the lumped/Floquet pairing.

- floquet_wave: open-ended waveguide array unit cell (2.5x2.5 cm periodic
  cell, 2x1 cm rectangular waveguide feed). TE10 mode propagates 8-11 GHz,
  single-mode on both port types. Verifies S-matrix unitarity and
  reciprocity for the wave/Floquet pairing.

Both cases include mesh generation scripts (Julia + Gmsh), Palace config,
and reference CSV data. Custom checks use the existing TestFloquetSParams
helper for port-floquet-S.csv (magnitude-only, NaN-tolerant).
simlapointe and others added 25 commits July 10, 2026 09:29
The CI arm64 runners use 64 MPI ranks. With only 128 elements (4x4x8),
some ranks get empty subdomains (0 vertices), causing a fatal abort in the
lumped port boundary setup. Increase to 432 elements (6x6x12) so every
rank has at least 6 elements even at 64 ranks.
These small meshes (432 and ~600 elements) don't partition cleanly at high
MPI rank counts (the CI arm64 runners use 64 ranks). With so few elements,
METIS can produce subdomains that have no faces on a port boundary, causing
the port operator setup to abort. The tests validate S-parameter power
normalization correctness, not parallel scaling — the existing
dielectric_grating_uniform test already covers Floquet ports in parallel.
…ning

The regression meshes were too small for the CI arm64 runners' 64 MPI
ranks: below ~7 elements per rank, METIS produces degenerate partitions
(subdomains with 0 vertices) which crash in parallel mesh/boundary setup
(segfault in the solve, or "non-periodic face cannot have multiple
boundary elements" in GetFaceToBdrElementMap). The failure is independent
of port type — it reproduces with plain PEC boundaries on the same mesh —
and disappears at ~2000+ elements.

Increase floquet_lumped to 8x8x20 (2112 elements) and floquet_wave to a
finer aperture discretization (3076 elements). Both now pass at 48, 56,
60, and 64 ranks. Serial runtimes remain modest (7 s and 24 s). Power
balance on the regenerated references holds to 1e-6.
Fix S-parameters when Floquet ports are used with lumped or wave ports
For pull_request events from forks, GITHUB_TOKEN is read-only regardless
of the workflow's declared permissions, so the peaceiris/actions-gh-pages
push to gh-pages fails with 403 and aborts the job before the regression
test steps run (observed on PR #770, the coverage matrix row).

Guard the deploy and the summary-link steps with the same same-repo
condition already used by docs.yml for its GHCR cache push. The coverage
report artifact upload is left unguarded since it works with a read-only
token, so fork PRs still get a downloadable coverage report.
Add a SurfaceRationalImpedanceOperator implementing a Robin boundary whose
per-square surface impedance is a user-specified rational function of
frequency, Zs(s) = N(s)/D(s) with s = iω. Numerator and denominator
polynomial coefficients (zeros and poles) are given as lists in the new
boundaries["RationalImpedance"] config block, allowing any passive
lumped-network response.

Being a general function of frequency, the boundary contributes iω/Zs(iω) to
the frequency-dependent system matrix A2(ω) (like SurfaceConductivityOperator),
and is available for the frequency domain driven problem type.
Coefficients are nondimensionalized as a_k/(Z0*tc^k) and b_k/tc^k.

Wiring spans configfile (RationalImpedanceData and parser), iodata
(nondimensionalization, problem-type warnings, nonlinear-eigenmode
sparsity-pattern handling), spaceoperator (assembly, auxiliary-space marker,
multiple-BC check), the JSON schema, and the docs.

Add a verification example (examples/rational_impedance) comparing, on a
parallel-plate TEM line, a parallel-RLC sheet realized as a RationalImpedance
against the same RLC realized as a LumpedPort; the two S11 responses agree to
solver tolerance.
…undary

Validate user-specified rational surface impedances and warn on physically
questionable inputs, following the Palace Mpi::Warning convention:

- Warn when the numerator/denominator degree difference exceeds one, since a
  positive-real (passive) impedance has |deg(N) - deg(D)| <= 1.
- Warn, once per boundary, when Re{Zs(iw)} < 0 at an evaluated frequency, with
  a relative tolerance so lossless reactive terminations do not trigger it.

Real-valued coefficients (enforced by the JSON schema and parser) guarantee a
Hermitian Zs(iw), and hence a real time-domain response.
Add the RationalImpedance boundary attributes to the aggregated boundary
attribute list built in BoundaryData. Without this, external rational-impedance
boundaries raised a spurious "no associated boundary condition, PMC assumed"
warning, and internal rational-impedance sheets were skipped by the
interface-element (mesh cracking) logic, so the boundary term was not applied
on them.
Co-authored-by: Layla Ghaffari <49916147+laylagi@users.noreply.github.com>
Add rational surface impedance boundary condition
@hughcars hughcars force-pushed the hughcars/pr717-cudss branch from 5f205a9 to fd5c2cb Compare July 13, 2026 16:25
Pennycook and others added 3 commits July 13, 2026 15:21
cuDSS is a high-performance CUDA library for Direct Sparse Solvers.

This commit enables the use of cuDSS in Palace by:
- Wrapping mfem::CuDSSSolver in a new palace::CuDSSSolver;
- Enabling the use of CuDSSSolver for KSP and WavePort; and
- Adding cuDSS configuration options to CMake, for both Palace and MFEM.
Maintainer feedback on the cuDSS support:

- Rebase cleanup: in cudss.cpp swap the transitive utils/iodata.hpp include for
  utils/labels.hpp, the header that actually defines the MatrixSymmetry /
  SymbolicFactorization types the file uses (include-what-you-use); drop the
  dead IoData forward-decl in cudss.hpp and the unused cudss.hpp include in
  waveportoperator.cpp; and document the intentionally-unused reorder argument
  in the CuDSSSolver constructor.

- Consolidate the "not built with cuDSS" guard to a single config-time check in
  iodata.cpp (mirroring the SuperLU/STRUMPACK/MUMPS guards), and drop the
  redundant #else MFEM_ABORT from the ksp.cpp cuDSS case so it matches the
  empty-guarded MUMPS pattern.

- Bump the configuration schema $id to 1-2-0 for the new "cuDSS"
  Solver.Linear.Type enum value: a new allowed enum value is a backward-
  compatible SchemaVer REVISION (required by scripts/check-schema-version). Tag
  the CHANGELOG entry with "SchemaVer 1-2-0" per the bump policy; the notes.md
  release table is intentionally left untouched (updated only at release time).

- Document cuDSS in the install guide (build flag + spack example) and add unit
  tests: every Solver.Linear.Type value (incl. "cuDSS") validates against the
  schema, and Type="cuDSS" parses to LinearSolver::CUDSS when built with cuDSS.

- Enable cuDSS in the GPU CI: build with +cuda+cudss and add a cuDSS entry to
  the GPU regression-test solver matrix.
@hughcars hughcars force-pushed the hughcars/pr717-cudss branch from fd5c2cb to 204ff5b Compare July 13, 2026 19:24
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.

5 participants