Fix false 'incomplete entity data' warning for HDF5 files - #18
Open
chraibi wants to merge 1 commit into
Open
Conversation
The incomplete-data warning compared the busiest timestep's agent count against MaxAgents, which false-positives whenever agents enter and leave over the run: the peak concurrent count is legitimately lower than the number of distinct entities. Track which entity IDs appear in any sample and warn only when some declared entities never show up at all.
There was a problem hiding this comment.
Pull request overview
This PR refines HDF5 “incomplete entity data” detection in AgentDataSubsystem to avoid false positives when entities enter/leave over time by switching from a peak-per-timestep comparison to a distinct-entity coverage check across all samples.
Changes:
- Removes the peak concurrent entity-count heuristic used for completeness warnings.
- Tracks which entity IDs appear in any HDF5 sample via
TBitArrayand warns only when some declared entities never appear. - Updates the warning text to report how many entities have no samples.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1323
to
+1332
| int32 ObservedEntityCount = 0; | ||
| for (const FHdf5SampleData& Sample : Hdf5Data.Samples) | ||
| { | ||
| if (ObservedEntities.IsValidIndex(Sample.EntityId) && !ObservedEntities[Sample.EntityId]) | ||
| { | ||
| ObservedEntities[Sample.EntityId] = true; | ||
| ++ObservedEntityCount; | ||
| } | ||
| } | ||
|
|
sir306
added a commit
that referenced
this pull request
Aug 13, 2026
The "Incomplete entity data" dialog fired on essentially every real
dataset. It compared PeakEntityCount against MaxAgents:
if (MaxAgents > 0 && PeakEntityCount > 0 && PeakEntityCount < MaxAgents)
PeakEntityCount is the largest number of entities present in any single
timestep -- a concurrency figure. MaxAgents is the number of distinct
entities in the whole run (ProcessMetadata takes either
Metadata.MaxNumEntities or Entities.Num()). Those are equal only if every
occupant is present at the same instant, so any staggered evacuation -- which
is what an evacuation is -- has peak < total and raised the warning while the
data was complete. A false error reads as "the tool is broken" before anyone
questions a number, which is why this went first.
The honest test is coverage: does every declared entity appear in at least
one timestep? Counted with one bit per entity rather than a TSet, since ids
are dense 0-based indices here -- the gathering loop above already indexes
AgentDataArray by EntityId -- so there is nothing to hash.
Two additions beyond the reported defect:
- An id at or above the declared count is now reported separately, as
"Unexpected entity IDs". It is the opposite problem (the file contains more
entities than it declares) and it has a different fix, so folding it into a
"data may be missing" message would mislead.
- Both reports are suppressed when bShouldStop. A cancelled import is
incomplete by request, and saying so in an error dialog is the same
false-alarm class this commit removes. The timestep check above is
deliberately left as it was rather than widened into this change.
Raised upstream as PR #18 by chraibi (PedestrianDynamics / JuPedSim), who
identified the comparison correctly. This is an independent implementation
rather than that patch merged, so the PR still needs closing.
Gate: editor target rebuilt with the editor closed -- 14 actions, zero
warnings, zero -NNNN hot-reload artifacts -- and the base
UnrealEditor-ProjectMobius.dll byte-scanned: all three new UTF-16 literals
present and the old "peak entity count" string absent, so the defect is gone
rather than shadowed. Then the full fire/smoke suite: pass 1 32/32, pass 2
16 ran / 15 passed. The single red is T_PIX_2_ExportAndRenderShareInputs, a
pre-existing heatmap-material failure unrelated to this file.
AI-assisted with Claude Code; all changes reviewed, tested, and verified by a
human maintainer in line with this repository's AI-tooling policy.
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
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.
The incomplete-data check compared the busiest timestep's agent count against
MaxAgents, which false-positives whenever agents enter and leave over the run — the peak concurrent count is legitimately lower than the number of distinct entities.This tracks which entity IDs appear in any sample (
TBitArray) and warns only when some declared entities never appear at all.Found while replaying JuPedSim HDF5 trajectories where the warning fired on every valid file.