YZmap update: jsonnet loading optimization#471
Open
HaiwangYu wants to merge 11 commits into
Open
Conversation
Member
Author
|
Msg from slack:
|
Member
Author
|
log from that run: |
brettviren
reviewed
May 19, 2026
| Log::set_pattern("[%H:%M:%S.%03e] %L [%^%=8n%$] %v"); | ||
| log = Log::logger("main"); | ||
| log->set_pattern("[%H:%M:%S.%03e] %L [ main ] %v"); | ||
| Log::set_level("debug", "main"); |
Member
There was a problem hiding this comment.
We should not set log levels in C++. They are for the end user to decide. Default level must remain "info".
brettviren
requested changes
May 19, 2026
Member
brettviren
left a comment
brettviren
left a comment
There was a problem hiding this comment.
Looks good except that the "main" log level should not be hard-wired in C++.
Member
Author
|
One related topic, for DUNE FD 10kt VD: 
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To fix this issue #469
Dev repo: https://github.com/HaiwangYu/wcdev-icarus
CI report: report-pr471.pdf
YZmap update: jsonnet loading optimization
Reduces peak heap and wall-clock during WCT job startup and event
processing for ICARUS detsim, and makes the
IConfigurable::configure()contract idempotent so multiple
WireCellToolkitart modules can shareWCT-side components in one art job (enables a per-TPC fcl that drops peak
RSS by another ~2.6 GB). Backwards compatible: with the standard 8-anode
config and a single
WireCellToolkitmodule, behavior is unchanged.Headline result on ICARUS detsim (1-event smoke test)
Cumulative VmHWM reduction: −3.8 GB single-module, −6.4 GB per-TPC
(−50% from master). CPU time unchanged.
What's in the PR
A. WCT configuration loading (
util/,apps/,cfg/)A series of independent fixes to the WCT-side config bootstrap that
together cut several GB of transient + resident heap during
Main::initialize()and the first few events.util/src/Persist.cxxjson2object()and the.jsonnetbranch ofParser::load()now parse straight from thechar*returned byjsonnet_evaluate_fileviaJson::CharReaderBuilder::newCharReader()::parse()instead of routing through astd::stringstream+ostringstream::str()pipeline. Eliminates ~3 redundant copies of the ~1 GB JSON text that the parser used to keep alive simultaneously. Also throwsIOErrorwith the parse error message on failure instead of silently returning an emptyJson::Value.util/inc/WireCellUtil/Configuration.h,util/src/Configuration.cxx,util/inc/WireCellUtil/ConfigManager.h,util/src/ConfigManager.cxx,apps/src/Main.cxxbranch()now takesconst Configuration&(plus a by-value overload kept for ABI compat with binaries built against the older signature on cvmfs). Walks the dot-path via const-ref instead of repeated self-assignment.•
find()andget()templates takeconst Configuration&.•
ConfigManager::all()returnsconst Configuration&instead of by-value.• Six
for (auto c : m_cfgmgr.all())/for (auto c : m_apps)loops inMain.cxx(lines 299, 312, 325, 331, 350, 375, 408 in the original) →for (const auto& …).•
ConfigManager::extend()andindex()useconst auto&iteration.• When a component's
default_configuration()is null, skip the no-opupdate()and passc["data"]straight toconfigure()(saves a deep-copy per component).util/src/ConfigManager.cxx,apps/src/Main.cxxConfigManager::extend()nowstd::move's each entry intom_topinstead of deep-copying. Signature staysvoid extend(Configuration more)to preserve ABI; new callers should passstd::move(cfg)(andMain.cxx:274does). Eliminates the per-entry deep copy inside the loop.util/src/ConfigManager.cxxConfigManager::pop()rewritten to useJson::Value::removeIndex(i, &ret)instead of cloning every other entry into a freshJson::arrayValueand reassigning it back overm_top. Drops another ~2 GB transient during thewire-cellentry extraction.util/inc/WireCellUtil/ConfigManager.h,util/src/ConfigManager.cxx,apps/src/Main.cxxConfigManager::clear_data()that drops the bulk"data"sub-tree from every top-level entry.Main::initialize()calls it after the configure loop completes;finalize()only consults"type"and"name"so the multi-GB residentJson::Valuetree is released before event processing begins. Lowers the post-config plateau ~1.8 GB and the late peak ~2 GB.The cumulative effect (single
WireCellToolkitmodule on theproduction ICARUS detsim fcl): VmHWM 12.8 GB → 9.0 GB (−3.8 GB), and
the configure phase finishes in ≈ 4 s of wall time instead of ≈ 80 s.
B.
IConfigurable::configure()idempotency fixes (gen/)WCT's
NamedFactoryis process-wide, so any setup that runsMain::initialize()more than once on the same process (most relevantly:multiple
WireCellToolkitart modules in one art job) callsconfigure()repeatedly on shared component instances. The contractrequires
configure()to be idempotent; three components weren't:PlaneImpactResponse::build_responses()appended tom_irandm_bywirewithout clearing first. After the secondreconfigure,
nwires()and the per-wire indexing drift out of syncwith
m_half_extent;PIR::closest()starts throwing for in-rangepitches and
ImpactTransformcrashes downstream. Fixed withm_ir.clear(); m_bywire.clear();at the top ofbuild_responses().DepoSetFilterYZ::configure()andScaler::configure()—same
m_boxes.push_back(face->sensitive())loop with no precedingclear(). Defensive fix, since their instances aren't currentlyshared between WCT art modules but the contract issue is the same.
Audit confirmed the rest of
gen/src/is already idempotent(
AnodePlane,MegaAnodePlane,DepoTransform,Reframer,YZMap,FieldResponse,ColdElecResponse,WarmElecResponse,JsonElecResponse,WireSchemaFileall clear or overwrite theirstate before populating).
C.
sim.jsonnetgeneralizationThree hardcoded
std.range(0, 359)incfg/pgrapher/experiment/icarus/sim.jsonnet(intransformsyz,reframersyz, andanalog_pipelinesyz) →std.range(0, nanodes*45 - 1).Each iterates over
tools.anodes[std.floor(n/45)]so a caller thatnarrows
tools.anodes(e.g. to a single-TPC pair viatools = tools_all { anodes: tools_all.anodes[apa_lo : apa_lo + 2] })now builds only the relevant 90 entries instead of indexing past the
end of the array. With the default 8-anode
tools.anodestheexpression evaluates to
(0, 359)and behavior is unchanged. This isthe change that enables the per-TPC fcl downstream.
D. ICARUS detsim jsonnet variant
cfg/pgrapher/experiment/icarus/wcls-multitpc-sim-drift-simchannel-yzsim-refactored.jsonnetships as the up-to-date refactored variant (uses the YZMap service, the
drifter_datadefinitions, and the FrameFanin→DumpFrames topologyadjustments that the production icarus detsim flow needs).
Why "memory" matters here
The original WCT-on-art configure path:
After this PR: ~2 GB transient gone from
extend, ~2 GB transient gonefrom
pop, ~3 GB transient gone from the parser's stringstreampipeline, the 6 by-value loops in
Main.cxxare by-ref, and theresident
m_toppayload is dropped afterconfigure()completes. Thecombined effect is a multi-GB drop in both the startup peak and the
post-config plateau.
Backwards compatibility
util/API changes are additive or maintain the old by-valuesignatures as ABI shims (declared in-place inside
namespace WireCellblocks in the.cxx, not in the headers, so new codeprefers the const-ref overload). Binaries built against the older
WCT headers on cvmfs (
libWireCellLarsoft,libWireCellAIML,libWireCellQLMatch, and the cvmfslibWireCell*themselves) linkcleanly. Verified with
nm -Dagainst the cvmfs build set.Pgrapher::configure()'s edge-accumulation behavior is unchanged.The per-TPC fcl side-steps it by using unique
Pgrapher:pgrapher<k>names.
sim.jsonnet'sstd.range(0, nanodes*45 - 1)evaluates to thelegacy
(0, 359)when called with the default 8-anode tools.wcls-multitpc-…-refactored.jsonnetis a separate variant;existing fcl that pointed at the older jsonnet keeps working.
Tests
detsim_2d_icarus_refactored_yzsim.fcl→ producesdetsim.root):RawDigit/SimChannelcollection byte-identical to master.WireCellToolkitart modules):passes, VmHWM 6.4 GB.
RawDigitinstance names preserved(
simdigits0..3).SimChannelinstance names preserved(
simpleSC0..359) but now distributed across the fourdaq0..daq3module labels.
gen/src/forpush_back-without-clearpatterns: onlyPIR / DepoSetFilterYZ / Scaler were affected, all addressed in this
PR. Others (
AnodePlane,MegaAnodePlane,DepoTransform,Reframer,YZMap, FieldResponse and the various*ElecResponse/WireSchemaFile) already clear or overwrite therelevant state.
Files
Commits