Skip to content

feat: add time-aware EDM support and realistic animations (fixes #406)#822

Open
deveshbervar wants to merge 2 commits intoHSF:mainfrom
deveshbervar:fix/issue-406-time-animations-v2
Open

feat: add time-aware EDM support and realistic animations (fixes #406)#822
deveshbervar wants to merge 2 commits intoHSF:mainfrom
deveshbervar:fix/issue-406-time-animations-v2

Conversation

@deveshbervar
Copy link
Copy Markdown
Collaborator

  • Add EventTime and EventMetadata interfaces to event-data-loader.ts
  • Add optional getEventTime?() to EventDataLoader interface
  • Extract event-level time (ns) in PhoenixLoader.buildEventData()
  • Forward event time to AnimationsManager
  • Add setEventTime(), getTimeProgress(), update() to AnimationsManager
  • Add Event Time Progress slider to dat-gui UI
  • Add tests for time extraction in phoenix-loader.test.ts

)

- Add EventTime and EventMetadata interfaces to event-data-loader.ts
- Add optional getEventTime?() to EventDataLoader interface
- Extract event-level time (ns) in PhoenixLoader.buildEventData()
- Forward event time to AnimationsManager
- Add setEventTime(), getTimeProgress(), update() to AnimationsManager
- Add Event Time Progress slider to dat-gui UI
- Add tests for time extraction in phoenix-loader.test.ts
@EdwardMoyse
Copy link
Copy Markdown
Collaborator

Hi @deveshbervar - sorry for taking so long to review this. I have to admit, I was a bit confused (and remain a bit confused) about that the intention is here. I don't really understand what eventTime represents, nor what this gains us?

@deveshbervar
Copy link
Copy Markdown
Collaborator Author

Hi @EdwardMoyse,

Thanks a lot for the review!

What eventTime is

eventTime (in nanoseconds) is optional event-level timing information from real or simulated data. It represents the total time span of the physics event (e.g. drift time, readout window, or time-of-flight).

This comes from the original request in #406 and the HallD event viewer experience: real timing (instead of purely visual animations) helps physicists quickly spot reconstruction bugs like wrong hit ordering or timing issues.

What this PR adds (minimal & non-breaking)

  • Optional EventTime support in the EDM (EventDataLoader + getEventTime?())
  • PhoenixLoader extracts eventData.time (if present) and forwards it to AnimationsManager
  • AnimationsManager gets:
    • setEventTime(timeNs)
    • getTimeProgress() → returns normalized progress [0, 1]
    • update(deltaSeconds) → advances internal time

Nothing changes for existing events or animations — this only introduces a small foundation for future time-driven (realistic) animations.

Next steps

I'm happy to simplify further:

  • Remove the dat-gui slider (added only for testing), or
  • Reduce this PR to EDM only and move the animation helpers to a follow-up.

What direction would you prefer?

@EdwardMoyse
Copy link
Copy Markdown
Collaborator

Sorry, another long delay. So some quick comments, eventTime seems to be treated more as a maxEventTime?

The way this would work is if we had timing information added to e.g. hits. Then as the animation progressed, objects would grow and become visible. As it is right now, we assume that there is an expanding wave of particles coming from the origin in a sphere and hits appear as the sphere hits them. This assumption is completely wrong if the detector is not symmetric around an origin and if the particles do not come from the collision. An example from ATLAS would be for cosmic rays, where they come from top to bottom, and if we included hit level (or track level) timing information this could be visualised better.

So as it stands, I'm not really sure that this PR is going in the right direction. And in particular, if eventTime is maxTime then this is a function of the detector rather than the event.

(We also don't really use dat.gui any more)

@deveshbervar
Copy link
Copy Markdown
Collaborator Author

Hi @EdwardMoyse,

Thank you for the detailed explanation — this is very helpful!

I understand now:

  • eventTime as a global value is wrong — it's effectively maxTime
    and belongs to the detector, not the event
  • The right approach is per-object timing (hit.time, track.time)
    so each object appears when the animation reaches its timestamp
  • The current sphere animation assumption breaks for
    non-symmetric detectors and cases like cosmic rays
  • dat.gui is deprecated so I should remove that part

Would you prefer I:

  1. Close this PR and open a new one focused only on adding
    optional time fields to individual hits/tracks in the EDM?
  2. Or continue in this PR with that direction?

I'm happy to start with just the EDM change (adding optional
time field to hit/track objects) as a foundation, without
touching the animation logic yet.

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.

2 participants