Concretize IoData defaults and write out resolved config by hughcars · Pull Request #719 · awslabs/palace

hughcars · 2026-04-30T17:54:22Z

Palace now drops config.json alongside palace.json in the output directory. Every implicit default and "Default" sentinel is filled in so the file alone is enough to reproduce the run.

IoData::ConcretizeDefaults takes the parsed config by value, fills keys the user left blank (and replaces "Default" sentinel strings) from the resolved IoData, and returns the result for main.cpp to stream to disk. User-provided entries pass through untouched.

A schema-driven coverage gate (SchemaCoverageGaps in test/unit/test-config.cpp) walks every section of the JSON schema and asserts that every optional property with a C++ default is emitted, modulo a small justified skip-list.

Supporting cleanup:

EigenSolverBackend::DEFAULT and TimeSteppingScheme::DEFAULT are now type-level aliases for their build-determined / canonical concrete value, so a default-constructed field is already at its concrete value and no runtime sentinel resolution is needed.
Solver.Eigenmode.MaxIts and MaxSize defaults are resolved inside the struct's JSON constructor: MaxIts = 1'000'000 (iteration cap only, no buffer), MaxSize = max(2*n, n+15) (SLEPc's Krylov-Schur formula; previously ARPACK silently used max(20, 2*n+1)). Same treatment for Solver.BoundaryMode.MaxSize and per-port Boundaries.WavePort[].MaxSize — these previously fed -1 into the SLEPc/ARPACK wrappers and segfaulted any caller (including the boundary-mode unit tests) that didn't trip a defensive fallback that no longer exists. All in-tree examples set these explicitly.
Regression harness asserts the config.json sidecar is produced for every example.

simlapointe

Overall LGTM. Tested it on many cases and it seems to work as expected.

A few small things:

This may be intended, but I find the AMR behavior slightly weird: config.json gets written in the iteration1 folder (but not subsequent iterations) and a symlink to it is created in the top-level output folder. Maybe having the actual file in the top-level output folder and not in any of the iteration* folders makes more sense?
Maybe unlikely, but if the input config is config.json and the user sets "Output": ".", then the resolved config overwrites the input config. Should we guard against this?
It would be nice if the resolved config would get printed when running --dry-run, but maybe that's asking for too much?
See comment below about the new max_size/num_vec default.

simlapointe · 2026-06-02T20:58:32Z

+  // Resolve subspace sentinel: fed directly into ModeEigenSolver → SLEPc/ARPACK.
+  if (max_size <= 0)
+  {
+    max_size = std::max(2 * mode_idx, mode_idx + 15);


Previously we were using max(20, 2 * mode_idx + 1), which is the SLEPc default, correct? I know the differences should be minor, but is there a particular reason to change to max(2 * mode_idx, mode_idx + 15)?

The old line was commented "Default from SLEPc," but that's a mislabel — max(20, 2*nev+1) isn't SLEPc's default; max(2*nev, nev+15) is. The two backends were silently resolving an unset MaxSize differently, so this unifies them on SLEPc's actual formula and makes the resolved config backend-independent. The cost is a slightly smaller default for small nev (e.g. nev=4: 20→19, nev=1: 20→16), though these stay well above nev and all in-tree examples set MaxSize explicitly, so nothing in the repo changes.

Sbozzolo

Thank you, really useful feature! I left some comments, but nothing very big. I tested with a bunch of our examples in the repo.

I am a little concerned with the amount of information that is represented in various different ways (from JSON to C++ to concretizer...), and I think this further motivates generating this code from a single source (not in this PR).

Sbozzolo · 2026-06-02T20:47:14Z

+  return config;
+}
+
+void IoData::WriteResolvedConfig(const json &raw_config) const


This will break the user's config is if <problem.output> = "." and the user's config is named config.json.

We might want to check if a file already exists before writing.

Fixed in 3466612.

Sbozzolo · 2026-06-02T20:58:58Z

+// DEFAULT is an alias for GEN_ALPHA; placing it last ensures to_json emits
+// "GeneralizedAlpha" rather than "Default" while still accepting "Default" as input.


I think this is a really great feature.

I tested it with some of our examples and found that there are still some "Defaults" left:

"ColumnOrdering": "Default", "PCSide": "Default",

These are intentionally left as "Default" — Palace delegates the choice to the downstream library (GMRES/FGMRES selects the PC side; the sparse direct solver selects the ordering), so there is no concrete value for Palace to emit. Documented as such in solver.md and postprocessing.md (3466612).

Sbozzolo · 2026-06-02T21:02:15Z

        for i = 1:max_its
            push!(expected_metafiles, "iteration$(i)/palace.json")
        end


this should probably also take into account the resolved config file (or we should avoid copying the resolved config file in each iteration)

Fixed in 3466612 — resolved config is now a single top-level <input>_resolved.json, no per-iteration copies, and the harness checks for it there.

Sbozzolo · 2026-06-02T21:03:15Z

  }
  nev = num_eig;
-  ncv = (num_vec > 0) ? num_vec : std::max(20, 2 * nev + 1);  // Default from SLEPc
+  ncv = num_vec;


Just noting that this changes the default when num_vec <= 4

Acknowledged — see the explanation on the configfile.cpp MaxSize thread. Both backends now resolve an unset subspace to SLEPc's actual default max(2*nev, nev+15); the change is deliberate and documented.

Sbozzolo · 2026-06-02T21:15:41Z

+  - Palace now writes a fully-resolved copy of the run configuration to
+    `<output_dir>/config.json` at startup, with every implicit default and `"Default"`
+    sentinel filled in. The sidecar passes schema validation and can be used to re-run
+    the same simulation deterministically [PR 719](https://github.com/awslabs/palace/pull/719).


We should document this in the docs as well.

Documented in 3466612 (new "Resolved configuration record" section in docs/src/guide/postprocessing.md, plus expanded PCSide/ColumnOrdering notes in solver.md).

Sbozzolo · 2026-06-02T21:39:27Z

+  }
+  if (max_size <= 0)
+  {
+    max_size = std::max(2 * n, n + 15);


Maybe we can add a function for this default, since we set it in multiple places?

Fixed in 3466612 — extracted DefaultEigenSubspaceSize(num_modes) and routed all three call sites through it.

Sbozzolo · 2026-06-02T21:56:19Z

+    CHECK(iodata.solver.linear.ams_singular_op == 1);
+    CHECK(iodata.solver.linear.ams_max_it == 1);
+    CHECK(iodata.solver.linear.mg_cycle_it == 1);
+    CHECK(iodata.solver.linear.pc_mat_sym == MatrixSymmetry::SPD);


MatrixSymmetry is an object implicitly defined and not a config. I don't think it should be in the solver.linear struct (which would otherwise represent the JSON faithfully).

Fixed in 032a091 — removed from LinearSolverData and derived at construction via GetPreconditionerMatrixSymmetry(iodata) in ksp.cpp.

Sbozzolo · 2026-06-02T22:07:58Z

+namespace palace
+{
+


I think we could remove this entire file by inlining the operator in PALACE_JSON_SERIALIZE_ENUM

I kept the file but addressed the underlying duplication. There is now one ToString/FromString mapping table per enum, and both the JSON to_json/from_json shims and the resolved-config writer delegate to it, so there is a single source of truth either way. I left a small declaration header rather than inlining into PALACE_JSON_SERIALIZE_ENUM for two reasons: the resolved-config writer needs ToString without pulling in the full nlohmann serialization machinery, and keeping the tables in a .cpp avoids re-expanding them at every macro site. (d22986c)

Sbozzolo · 2026-06-02T22:10:50Z

+  }
+}
+
+void ConcretizeProblem(const config::ProblemData &problem, json &j_problem)


In the future, this entire file could (should?) probably be generated from the schema in the spirit of #723

Agreed, and #723 is the right vehicle. This hand-written concretizer plus the schema coverage gate is the stopgap until then — I'd keep codegen out of this PR's scope. The coverage test fails loudly if a schema property ever drifts out of sync in the meantime.

hughcars · 2026-06-08T13:40:14Z

Current spack CI failures will be fixed once rebased on top of #742 once that is merged.

…figuration Make DEFAULT a distinct enum value and resolve it to the concrete backend during CheckConfiguration so drivers never observe the sentinel.

…Configuration Fold the PETSC_DEFAULT / std::max(20, 2*n+1) fallbacks from the SLEPc and ARPACK wrappers into CheckConfiguration, and pick Palace defaults (max_it = 1'000'000, max_size = max(2*n, n+15)) so IoData is concrete post-construction.

Write a fully-resolved copy of the configuration to <stem>-resolved.json in the output directory — every implicit default filled in so the file alone is enough to re-run the simulation. IoData::ConcretizeDefaults takes the parsed config by value, fills any missing keys (and replaces "Default" sentinel strings) from the resolved IoData, and returns it for main.cpp to stream to disk. Build-availability guards for SLEPc/ARPACK/SuperLU/STRUMPACK/MUMPS are centralised in CheckConfiguration. IoData gains a static ParseAndValidate so main.cpp can feed one JSON to both construction and concretization.

Palace drops <stem>-resolved.json next to palace.json in the output directory. Normalise the tempname-derived stem to a fixed "-resolved.json" key and add it to expected_metafiles so the harness asserts the sidecar is produced.

Every defaulted field in ModelData, RefinementData, FarfieldBoundaryData, ConductivityData, ImpedanceData, LumpedPortData, WavePortData, PeriodicBoundaryData, and the Boundaries.Postprocessing sub-structs is now written to the resolved config.

Signed-off-by: Hugh Carson <hughcars@amazon.com>

…ter" This reverts commit 9954bcb.

…ation Signed-off-by: Hugh Carson <hughcars@amazon.com>

Signed-off-by: Hugh Carson <hughcars@amazon.com>

…ration Signed-off-by: Hugh Carson <hughcars@amazon.com>

Signed-off-by: Hugh Carson <hughcars@amazon.com>

- Name resolved config <input>_resolved.json to avoid overwriting input - Keep resolved config at top level during AMR (skip iteration sweep) - Update regression harness to expect top-level _resolved.json - Print resolved configuration on --dry-run - Factor out DefaultEigenSubspaceSize helper for eigen subspace default - Document PCSide/ColumnOrdering as library-delegated defaults - Document resolved-config sidecar in postprocessing guide and CHANGELOG

…ig struct pc_mat_sym was a derived runtime property stored on LinearSolverData, which should mirror the JSON config faithfully. Move the derivation into a free GetPreconditionerMatrixSymmetry(IoData) helper used at Ksp construction, thread the resulting MatrixSymmetry through the linear-only BaseKspSolver ctor, and remove the field. Also drops the now-dead MumpsSolver(IoData,...) ctor.

…root The concretizer's EnumString/operator<< was a second enum->string mechanism redundant with nlohmann to_json. Move the one mapping table per enum into enum_string.cpp behind ToString/FromString; the JSON to_json/from_json shims and the resolved-config writer now delegate to it. Removes operator<<, EnumString, and the triple-duplicated mapping arrays in the macro.

Give `Problem.Output` a concrete `"postpro"` defaulter Requires #719 Closes #745

hughcars force-pushed the hughcars/concretize-defaults branch 3 times, most recently from dffb2d0 to 2170b75 Compare May 11, 2026 18:29

hughcars force-pushed the hughcars/concretize-defaults branch 2 times, most recently from 6dbf12c to 4baca74 Compare May 14, 2026 17:37

hughcars added the no-long-tests This PR does not require the long tests to be merged label May 14, 2026

hughcars requested a review from Sbozzolo May 15, 2026 18:12

hughcars force-pushed the hughcars/concretize-defaults branch from dfda2c5 to 592644b Compare May 15, 2026 18:18

This was referenced May 26, 2026

Auto-gen config docs #716

Merged

schema: hoist inline definitions into named $defs for codegen #726

Merged

parrangoiz mentioned this pull request Jun 2, 2026

Runtime behavior contradicts schema #745

Closed

simlapointe requested changes Jun 2, 2026

View reviewed changes

Sbozzolo requested changes Jun 2, 2026

View reviewed changes

hughcars force-pushed the hughcars/concretize-defaults branch 2 times, most recently from 82b2575 to d22986c Compare June 5, 2026 13:14

hughcars requested review from Sbozzolo and simlapointe June 8, 2026 13:37

simlapointe approved these changes Jun 8, 2026

View reviewed changes

Sbozzolo approved these changes Jun 8, 2026

View reviewed changes

Sbozzolo mentioned this pull request Jun 8, 2026

Default Problem.Output to "postpro" when omitted #753

Merged

hughcars force-pushed the hughcars/concretize-defaults branch from d22986c to 2ea63c6 Compare June 8, 2026 18:04

hughcars enabled auto-merge June 8, 2026 18:04

hughcars disabled auto-merge June 8, 2026 18:54

hughcars enabled auto-merge June 8, 2026 19:50

hughcars added 5 commits June 8, 2026 16:58

Resolve EigenSolverBackend and TimeSteppingScheme DEFAULT in CheckCon…

f964488

…figuration Make DEFAULT a distinct enum value and resolve it to the concrete backend during CheckConfiguration so drivers never observe the sentinel.

Write resolved config as "config.json" and trace the write

9ea3874

hughcars added 25 commits June 8, 2026 16:58

Match WriteMetadata pattern for resolved config write, verify on disk

62be1d7

Dump postpro listing and stdout on metafile mismatch

4807679

Revert postpro-listing diagnostic and reflow MFEM_VERIFY

758a76b

Concretize BoundaryMode solver block

2913a20

Round-trip-test Model, Refinement, and Boundaries concretization

7494dcd

Restore sentinel defaults in SLEPc/ARPACK SetNumModes/SetMaxIter

c46b2e4

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Skip Lc and scheme-specific transient fields in ConcretizeDefaults

6c4ad35

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Revert "Restore sentinel defaults in SLEPc/ARPACK SetNumModes/SetMaxI…

377a558

…ter" This reverts commit 9954bcb.

Resolve BoundaryMode and WavePort max_size sentinels in CheckConfigur…

f60f767

…ation Signed-off-by: Hugh Carson <hughcars@amazon.com>

Pass concrete num_vec to ModeEigenSolver in boundary-mode unit tests

354b2db

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Resolve eigenvalue subspace sentinels inside parser, not CheckConfigu…

4fa79de

…ration Signed-off-by: Hugh Carson <hughcars@amazon.com>

Round-trip test for BoundaryMode defaulted fields

61afe33

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Round-trip test for WavePort defaulted fields

2c45026

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Emit scalar baselines for MaterialData physical properties

b19f201

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Apply clang-format to iodata_concretize.cpp

ed80d38

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Drop SuperLU from test fixtures so CI runners without it can run

3394e03

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Coverage gate: assert Concretize emits every optional schema property

9bd392d

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Make DEFAULT enum values aliases for their concrete resolution

0318b16

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Extract WriteResolvedConfig and tidy ConcretizeDefaults helpers

2d0f14a

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Move concretize-defaults changelog entries to In progress

06e62f0

Signed-off-by: Hugh Carson <hughcars@amazon.com>

Fixes for rebasing IncludeInSynthesis

c9d909b

hughcars force-pushed the hughcars/concretize-defaults branch from 8cf56cc to c9d909b Compare June 8, 2026 21:11

hughcars merged commit d255a88 into main Jun 8, 2026
67 of 68 checks passed

hughcars deleted the hughcars/concretize-defaults branch June 8, 2026 22:35

Sbozzolo added a commit that referenced this pull request Jun 8, 2026

Default Problem.Output to "postpro" when omitted

9843845

Give `Problem.Output` a concrete `"postpro"` defaulter Requires #719 Closes #745

Sbozzolo added a commit that referenced this pull request Jun 8, 2026

Default Problem.Output to "postpro" when omitted

3574e5a

Give `Problem.Output` a concrete `"postpro"` defaulter Requires #719 Closes #745

		// DEFAULT is an alias for GEN_ALPHA; placing it last ensures to_json emits
		// "GeneralizedAlpha" rather than "Default" while still accepting "Default" as input.

		namespace palace
		{

Uh oh!

Conversation

hughcars commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simlapointe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sbozzolo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hughcars commented Jun 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hughcars commented Apr 30, 2026 •

edited

Loading

Sbozzolo left a comment •

edited

Loading