Support multiple CEP stimuli per domain#586
Conversation
michelebucelli
left a comment
There was a problem hiding this comment.
Thank you @samibismar! I have only one small comment below but otherwise this looks good.
Beyond that, I think this PR could be a good time to make sure that the new features (introduced here and in #579) are covered by the integration tests. I think it would be good to do it without introducing new tests, if possible - do you know if any of the cep tests have multiple stimuli, currently applied with multiple domains?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #586 +/- ##
==========================================
+ Coverage 72.33% 72.46% +0.12%
==========================================
Files 241 241
Lines 38632 38646 +14
Branches 6641 6644 +3
==========================================
+ Hits 27944 28004 +60
+ Misses 10453 10407 -46
Partials 235 235 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thank you @michelebucelli, I replied to your inline comment about the use of I found that My thought is to merge the current Domain 3 region into Domain 2 by relabeling the domain IDs, remove the separate The current integration test only runs one time step, so the second stimulus at Does this sound like the approach you had in mind? |
|
@samibismar I think your suggestion makes sense. I would leave the test setting exactly as it is, even if the second stimulus doesn't fire. The code paths for multiple stimuli will still be used to some extent. What I would do is the following:
|
|
Thanks @michelebucelli, that makes sense. I’ll generate a longer-run solution with the current test setup, convert the case to use two spatially bounded stimuli within one domain, and compare the long-run solutions. I’ll then restore the original short test duration and verify it through |
|
@michelebucelli We do need to think about refactoring |
|
@michelebucelli I updated I ran the original and modified cases for 100 time steps and compared the outputs every 10 steps. The mesh and all saved solution data, including I then restored the original one-step test duration and verified the case through |
michelebucelli
left a comment
There was a problem hiding this comment.
Thank you @samibismar! The updated test looks good to me.
I agree! I have some half-formed thoughts about this, stemming from other refactoring work (most notably #540 and #578). If you want, I can open an issue about it so we can start discussing this more concretely. I'd love to hear your and @zasexton's thoughts about it (and I don't know if @zasexton's planned refactoring also includes parameters or not). |
|
I'd be interested to hear your thoughts @michelebucelli . I won't be immediately touching the |
|
@michelebucelli Your half-formed thoughts are welcome, perhaps superior to my fully-formed thoughts ... At the least the classes in Parameters.h,cpp need to be broken out into separate files into some sort of io directory. Please open an Issue for this. |
|
@ktbolt, should I open a follow-up issue in |
|
@samibismar Please update the |
|
@ktbolt, I opened SimVascular/simvascular.github.io#102 with the documentation update. |
Current situation
Closes #581.
This PR allows multiple CEP
<Stimulus>elements to be defined within the same domain. Each stimulus is parsed, stored, distributed, and evaluated independently, including its own amplitude, timing parameters, and optional spatial bounds.This allows spatially separated regions within the same CEP domain to be stimulated without creating additional domains solely for stimulus placement.
Existing inputs containing zero or one
<Stimulus>preserve their previous behavior.Release Notes
stimTypewith a collection of runtime stimuli.<Stimulus>elements under both:<Domain>Example:
Documentation
This feature uses the existing
<Stimulus>XML structure and allows it to be repeated within the same CEP domain. No new stimulus parameters are introduced.A follow-up update will be needed in the svMultiPhysics documentation.
Testing
Successfully compiled on macOS.
Validated using AP Niederer CEP benchmark cases covering:
All expected comparisons passed in serial and with MPI using 2 processes.
A visual validation was also run with 4 MPI processes using two spatially separated stimuli within the same domain. The results showed two distinct initial activation sites followed by propagation from both regions.
Code of Conduct & Contributing Guidelines