Reduce memory usage of sample-level result accumulators - #109
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #109 +/- ##
==========================================
+ Coverage 84.52% 84.57% +0.04%
==========================================
Files 45 45
Lines 2592 2658 +66
==========================================
+ Hits 2191 2248 +57
- Misses 401 410 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
f6313c9 to
b4b5ba3
Compare
b113d5e to
8adce48
Compare
|
|
||
| sampledata(acc::LineAvailabilityAccumulator) = acc.available | ||
|
|
||
| accumulatortype(::LineAvailability) = LineAvailabilityAccumulator |
There was a problem hiding this comment.
Why is accumulatortype only defined for LineAvailability? Was this not defined to begin with?
There was a problem hiding this comment.
accumulatortype(::LineAvailability) was already defined before this PR. Each Resultspec defines its corresponding accumulatortype method in its own source file. The only new method this PR added here is sampledata(::LineAvailabilityAccumulator)
| import ..Results | ||
| import ..Results: ResultSpec, ResultAccumulator, | ||
| accumulator, resultchannel, finalize | ||
| accumulator, finalize, resultchannel, usesamplepartitions |
There was a problem hiding this comment.
usesamplepartitions is internal API, right?
There was a problem hiding this comment.
That’s correct and it is not exported from Results. Simulations.jl imports it only to determine whether each worker accumulator should be allocated for its local sample partition or for the complete sample count.
| else | ||
| assess(system, method, sampleseeds, results, resultspecs...) | ||
| if method.threaded && threads == 1 | ||
| @warn "It looks like you haven't configured JULIA_NUM_THREADS before you started the julia repl. \n If you want to use multi-threading, stop the execution and start your julia repl using : \n julia --project --threads auto" |
There was a problem hiding this comment.
Should we also change this warning to reflect the doc string?
There was a problem hiding this comment.
I guess what I was saying here is: threaded::Bool=true`: Enable threaded Monte Carlo simulation is included in the docs. Should we just rephrase this warning to say the same? To enable threaded Monte Carlo ... I will resolve this for now.
97dc530 to
0151207
Compare
Previously, threaded execution created worker-local sample accumulators sized for the full Monte Carlo sample count. For sample-based result specs like
ShortfallSamplesthis caused memory usage to scale asO(regions x timesteps x samples x threads). After theCVARimplementation added per-sample shortfall totals,Shortfallwas affected as well.This PR changes sample-based result accumulation so each threaded worker's recorder stores only that worker's assigned sample range. The partitions are then copied into their corresponding positions during finalization to produce the complete full-size sample result. Non-sample statistics continue to be merged as before.
Example for 3 threaded workers:
Before, each worker's sample accumulator allocated the full sample dimension:
Now, samples are split into ranges:
Benchmarks
System: Guam 2028, 13 regions, 8760 timestamps, hourly resolution
Simulation: Run on HPC, using
standardnodes (104 cores, 250 GB)Result:
ShortfallSamples()1000 MC Samples
10000 MC Samples