Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Benchmark 7 of Aubry et al. (2022) as a Scenario2 variant #151

Merged
merged 3 commits into from
Sep 26, 2023

Conversation

charlesbmi
Copy link
Contributor

@charlesbmi charlesbmi commented Aug 31, 2023

Introduction

NDK's simulation scenarios are based on Jean-Francois Aubry, et al. (2022). Specifically, Scenario 1 corresponds to Benchmark 4 in the paper, and Scenario 2 corresponds to Benchmark 8 in the paper.

NDK's 3-D simulations run much faster on a GPU, but at standard resolutions, Scenario 2 (i.e., benchmark 8) is too large to fit on a single GPU's memory. Aubry, et al. (2022) also mention a related Benchmark 7, which is the same set up as benchmark 8 but simulated over a sub-volume of the skull/brain. Benchmark 7 uses the same volume size as Benchmark 4 and thus fits in a GPU.

Benchmark 7 would thus be useful as an easier-to-run alternative to Benchmark 8.

Changes

  • Add Scenario2_2D_Benchmark7 as a subclass of Scenario2_2D
  • Add Scenario2_3D_Benchmark7 as a subclass of Scenario2_3D
  • Check in the skull mask for Benchmark 7
  • Load in the appropriate skull mask file (Benchmark 8 vs 7) for the given scenario type.
Design decisions:
  • Made the new scenarios as subclasses of the existing Scenario2
    • Alternative considered: use extent variable to subslice the existing Scenario. Con: resulting code was less straightforward
    • Alternative considered: create entirely new Scenario class. Con: more places to update/maintain code if refactoring in the future

Testing

  • Can initialize Scenario2_3D_Benchmark7 in your code

  • Can adapt plot_scenarios.py (did not check in this change) to include Scenario2_3D_Benchmark7 and see results:
    image
    output

  • New scenarios added to test_builtin.py

@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2023

Codecov Report

All modified lines are covered by tests ✅

Files Coverage Δ
src/neurotechdevkit/scenarios/built_in/__init__.py 100.00% <100.00%> (ø)
.../neurotechdevkit/scenarios/built_in/_scenario_2.py 98.88% <100.00%> (+0.38%) ⬆️

📢 Thoughts on this report? Let us know!.

@charlesbmi charlesbmi self-assigned this Aug 31, 2023
@charlesbmi
Copy link
Contributor Author

charlesbmi commented Sep 25, 2023

Per discussion with Diogo: let's rename these scenarios to something meaningful, e.g. something like "FullSkull" or "3-layer model"

Made a ticket for this here: #156

I will fix that in a different PR.

@charlesbmi
Copy link
Contributor Author

Slack-approval on this one from Diogo - merging.

@charlesbmi charlesbmi merged commit 5b34206 into main Sep 26, 2023
@charlesbmi charlesbmi deleted the 89-aubry-benchmark-7-scenario branch September 26, 2023 21:02
NewtonSander pushed a commit that referenced this pull request Jul 26, 2024
* Add Scenario2 variant that mirrors Benchmark 7 of Aubry et al. (2022)

* Fix spellcheck
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a notebook that replicates the setup and results of the benchmark 7 of Jean-Francois Aubry et al paper
3 participants