Skip to content

Optimize 4D timeseries performance and add interactive timeseries panel - #745

Open
jwparks wants to merge 1 commit into
gallantlab:mainfrom
jwparks:feat/timeseries-viewer
Open

jwparks wants to merge 1 commit into
gallantlab:mainfrom
jwparks:feat/timeseries-viewer

Conversation

@jwparks

@jwparks jwparks commented Sep 20, 2026

Copy link
Copy Markdown

Resolves #730

While pycortex previously supported 4D data, the implementation was somewhat inefficient and slow. This PR improves the overall speed and algorithms for processing 4D timeseries data and adds a new time-series panel to the pycortex viewer. Additionally, I've included two example scripts to demonstrate these changes. Please take a look and review! #

Added files

These are example scripts to demonstrate timeseries functionalities

  1. examples/timeseries/README.txt
  2. examples/timeseries/encoding_model_timeseries.py
  3. examples/timeseries/rgb_timeseries.py

Edited files

  1. cortex/dataset/braindata.py
    min/max in to_json (_nan_to_num_bounds() on line 714)
    The original calls np.nan_to_num(data).min() and then .max(). nan_to_num copies the whole array, so this makes two full copies just to read two numbers. For 4D movies, that's several GB allocated, which was a large part of the viewer's startup time. I changed it to walk the array one slice at a time and keep a running min/max. The result is identical because nan_to_num is elementwise and min/max are associative, but only one slice is in memory at a time and the data is converted once instead of twice.
    hashing
    The original hashes array.tobytes(), which also copies the whole array first (which is the slow part for 4D data). I changed it to hash the array's buffer directly through a memoryview.

  2. cortex/webgl/data.py
    Packaging renders every frame of a 4D dataset into a PNG mosaic before show() returns. The original implementation took several minutes to load about 500 volumes of a run before the server started. I added a lazy=True mode that packs only frame 0 up front and packs the rest in get_image() when the browser or user asks (e.g., clicking the "timeseries" button). The viewer now opens in seconds instead of minutes.

  3. cortex/webgl/view.py
    New /timeseries endpoint
    I added a handler that slices the timecourse out of the array Python already holds and returns JSON.
    Startup order of show()
    Original show() (e.g., webshow()) did all packaging and surface cache work before printing the URL, so the terminal looked frozen on 4D data. I start the server and print the URL first, then run the heavy background work in _prepare(), with handlers waiting on a threading.Event.
    1D timeseries traces (for design matrix / regressors)
    I pull plain 1D ndarrays out of the show() dict and return them with each timeseries response, so you can pass {"run": vol, "stimulus": regressor} and overlay them.

  4. cortex/webgl/resources/js/dataset.js
    A movie pulls every frame into the browser on page load (requiring a long wait for all volumes). I changed it to load volume (frame) 0 first, stream the rest in the background, and use setPriority() to re-aim the stream at the frame the user jumps to.

  5. cortex/webgl/resources/js/mriview.js
    I added a timeseries button to the movie controls, wired picks to fetch from /timeseries, and added seekFrame() so clicking a timepoint moves the brain. dataBuffersReady now checks that frame 0 exists rather than checking array length, since the texture array is sparse.

  6. cortex/webgl/resources/js/figure.js
    I added a canvas panel for timeseries with a checkbox, a color picker, and a raw / z-scored toggle button.

@jwparks
jwparks requested a balanced review from Copilot September 20, 2026 04:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@jwparks

jwparks commented Sep 20, 2026

Copy link
Copy Markdown
Author

@claude review

@claude

claude Bot commented Sep 20, 2026

Copy link
Copy Markdown

Claude finished @jwparks's task in 9m 56s —— View job


Claude's Review

Tasks

  • Gather context (PR description, changed files list)
  • Fetch and inspect the diff against origin/main
  • Review cortex/dataset/braindata.py changes (min/max, hashing)
  • Review cortex/webgl/data.py (lazy packaging)
  • Review cortex/webgl/view.py (startup order, /timeseries endpoint, threading)
  • Review JS changes (dataset.js, mriview.js, figure.js)
  • Review example scripts
  • Run relevant test suite

Nice PR overall — the memory-copy reductions in braindata.py are correct and well-reasoned, and the lazy-packing / streaming design for 4D movies is a solid approach to the startup-time problem. test_braindata.py, test_dataset.py, and test_webgl_data.py all pass locally against this branch (the headless-browser suite takes longer than my sandbox allows to finish, so I wasn't able to confirm it end-to-end — worth double-checking CI for test_webgl_headless.py/test_headless.py).

Findings

1. _prepare() doesn't actually run in the background — cortex/webgl/view.py:420,1151
The PR description says startup work now runs "in the background... with handlers waiting on a threading.Event," but _prepare() is called directly and synchronously (_prepare() then _ready.set()), never on a threading.Thread. Since open_browser defaults to true (cortex/defaults.cfg:178), and webbrowser.open(url) only happens after _prepare() returns, the default/common path gets no benefit from the _ready machinery — the browser still only opens once all prep work is done, same as before. The genuine "seconds instead of minutes" win here comes from the lazy=True Package changes in data.py, not from this threading scaffolding. The only case where printing the URL early actually matters is display_url=True with open_browser=False, which is a narrower path than the PR description implies.

2. Blocking threading.Event.wait() inside synchronous Tornado handlers stalls the whole server
WebApp is a threading.Thread running a single-threaded Tornado IOLoop (cortex/webgl/serve.py:297). The new handlers (CTMHandler, DataHandler, MixerHandler, PickerHandler, TimeseriesHandler, StimHandler) call _ready.wait() directly inside synchronous get() methods. A blocking wait like this, executed on the IOLoop thread, will freeze the entire event loop — not just that one request — for any client that connects before _prepare() finishes (e.g., a second browser tab, or a script that grabs the printed URL and connects immediately). /static/* is not gated the same way, but mixer.html itself is, so an early visitor just gets a hung connection rather than a page that loads progressively. If real concurrency during startup is wanted, _prepare() should run on its own thread (started right after server.start()/the print statements) and handlers should use an async-friendly wait (e.g., tornado.locks.Event) rather than blocking the reactor thread. If concurrent access during prep isn't actually a goal, the _ready/threading.Event scaffolding could be dropped in favor of just reordering the existing calls.

3. Lazy packaging doesn't cover VolumeRGB movies — cortex/webgl/data.py:36-46
For VolumeRGB/VertexRGB, encdata.astype(np.uint8) (plus, for VertexRGB, the premultiply step) runs unconditionally before the lazy check, i.e. it always copies/processes the whole array up front regardless of lazy=True. The subsequent mosaic/PNG packing is deferred correctly for VolumeRGB (it falls into the elif lazy: branch), but the initial full-array uint8 cast is not, so a large 4D VolumeRGB movie won't get the same startup-time win as a plain Volume movie. Worth confirming this is an accepted scope limitation (the included examples only use VertexRGB, which is inherently small/vertex-sized) rather than an oversight — if VolumeRGB movies are meant to benefit too, that .astype(np.uint8) call needs to move behind the same per-frame path.

Minor / non-blocking

  • _nan_to_num_bounds (braindata.py:714) and the _hash memoryview change both check out — verified they're byte-for-byte equivalent to the old tobytes()/double-nan_to_num behavior (confirmed by the existing test_hash_uses_tobytes test, which still passes), just without the extra full-array copies.
  • CRLF line endings in dataset.js are preserved correctly (all 581 lines still end \r\n), per the repo convention noted in AGENTS.md.
  • Nice touch keeping the example scripts self-contained (synthetic data, no external downloads) and documenting the +/- key + panel workflow.

None of these are blockers for the core approach, but #1/#2 mean the "server responsive during startup" story in the description doesn't quite match what the code does yet, and #3 is a gap worth a conscious decision either way.

This branch has not been deployed

No deployments
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.

add PR of pycortex timeseries feature

2 participants