compartment sets (corenrn)#3542
Merged
Merged
Conversation
Contributor
|
✔️ 07537fd -> artifacts URL |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3542 +/- ##
==========================================
+ Coverage 68.45% 68.46% +0.01%
==========================================
Files 685 685
Lines 116809 116720 -89
==========================================
- Hits 79964 79917 -47
+ Misses 36845 36803 -42 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
✔️ ae12f47 -> Azure artifacts URL |
|
✔️ deb88c4 -> Azure artifacts URL |
|
✔️ 6cc6885 -> Azure artifacts URL |
This was referenced Aug 8, 2025
Merged
|
✔️ a3f8170 -> Azure artifacts URL |
|
✔️ bdb1d43 -> Azure artifacts URL |
|
✔️ 9cd1774 -> Azure artifacts URL |
|
✔️ 99def07 -> Azure artifacts URL |
|
✔️ 4b6ef68 -> Azure artifacts URL |
|
✔️ 8fb9c99 -> Azure artifacts URL |
|
✔️ 3150c6d -> Azure artifacts URL |
|
✔️ a686c93 -> Azure artifacts URL |
|
✔️ b94f686 -> Azure artifacts URL |
nrnhines
approved these changes
Aug 21, 2025
Member
Author
|
the merge of |
|
✔️ 3e7ba4e -> Azure artifacts URL |
Contributor
|
✔️ 0871d38 -> artifacts URL |
Contributor
|
✔️ 3e7ba4e -> artifacts URL |
b94f686 to
26d0828
Compare
26d0828 to
0d51ee6
Compare
|
|
✔️ ad20974 -> Azure artifacts URL |
Contributor
|
✔️ ad20974 -> artifacts URL |
cattabiani
added a commit
to openbraininstitute/neurodamus
that referenced
this pull request
Sep 4, 2025
## Context This should finally take care of the compartment sets for coreneuron Fix: #345 #322 testing for: neuronsimulator/nrn#3507 neuronsimulator/nrn#3542 ## Scope - [x] rework report handling (mostly in node and target_manager this time. i.e. removal of `_report_setup`) - [x] new `CoreReportConfig` to finally handle in a proper way `report.conf` - [x] remove `write_sim_config`, `update_report_conf`, `write_report_conf` and similar functions in `CoreConfig`. Use `CoreSimulationConfig` and `CoreReportConfig` instead - [x] move `cell-permute` in another issue/pr. EDIT done: #367 - [x] scaling moved to `report_parameters.py` - [x] `CumulativeError` moved in `pyutils` - [x] `ReportParams` now has its own file alleviating the burden of the `node.py` file. `_report_build_params` (previously in `node.py`) is also inside the new file. - [x] `target_type` is no more. We do not need encoding/decoding. We just write `sections` and `compartments` in the `report.conf` - [x] streamlined `enable_reports`. It is still a complex function but it should be easier to read and with less edge cases - [x] use the decorator `@cache_errors` to just fill a `CumulativeError` instead of raising an error and raise it at the end - [x] `merge_dicts` (in `tests/utils.py`) allows for overrides and suppression of sections from the child dict with the special keywords: `override_field` and `delete_field` ### `report.conf` changes - [x] `type_name` has been split into `sections` and `compartments`. No need to encode everything in an enum to decode later. This simplifies code and makes the `report.conf`. The code is slightly slower `0(1)` in a part of the code that was not performance-critical - [x] `scaling` added at the end of the report line (after buffer size) - [x] add 2 additional binary lines in case of a compartment set report to pass the exact positions of the compartment sets. They are necessary if and only if the report type is `compartment_set`. Having them when it is not or not having them when it is is an error. ## Testing - [x] add integration-e2e tests in `test_reports.py` ## Testing - [x] test various bug fixes in coreneuron - [x] add lfp test. Check that it fails with neuron - [x] fix integration-e2e - [x] fix scientific - [x] fix unit
NghiV1412
pushed a commit
to ABL-Lab/neurodamus
that referenced
this pull request
Dec 18, 2025
## Context This should finally take care of the compartment sets for coreneuron Fix: #345 #322 testing for: neuronsimulator/nrn#3507 neuronsimulator/nrn#3542 ## Scope - [x] rework report handling (mostly in node and target_manager this time. i.e. removal of `_report_setup`) - [x] new `CoreReportConfig` to finally handle in a proper way `report.conf` - [x] remove `write_sim_config`, `update_report_conf`, `write_report_conf` and similar functions in `CoreConfig`. Use `CoreSimulationConfig` and `CoreReportConfig` instead - [x] move `cell-permute` in another issue/pr. EDIT done: #367 - [x] scaling moved to `report_parameters.py` - [x] `CumulativeError` moved in `pyutils` - [x] `ReportParams` now has its own file alleviating the burden of the `node.py` file. `_report_build_params` (previously in `node.py`) is also inside the new file. - [x] `target_type` is no more. We do not need encoding/decoding. We just write `sections` and `compartments` in the `report.conf` - [x] streamlined `enable_reports`. It is still a complex function but it should be easier to read and with less edge cases - [x] use the decorator `@cache_errors` to just fill a `CumulativeError` instead of raising an error and raise it at the end - [x] `merge_dicts` (in `tests/utils.py`) allows for overrides and suppression of sections from the child dict with the special keywords: `override_field` and `delete_field` ### `report.conf` changes - [x] `type_name` has been split into `sections` and `compartments`. No need to encode everything in an enum to decode later. This simplifies code and makes the `report.conf`. The code is slightly slower `0(1)` in a part of the code that was not performance-critical - [x] `scaling` added at the end of the report line (after buffer size) - [x] add 2 additional binary lines in case of a compartment set report to pass the exact positions of the compartment sets. They are necessary if and only if the report type is `compartment_set`. Having them when it is not or not having them when it is is an error. ## Testing - [x] add integration-e2e tests in `test_reports.py` ## Testing - [x] test various bug fixes in coreneuron - [x] add lfp test. Check that it fails with neuron - [x] fix integration-e2e - [x] fix scientific - [x] fix unit
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
2 participants
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.
Why
This is the continuation of this #3507 to enable
CompartmentSetreports in coreneuron. They are nested on top ofCompartmentreports. In fact, conceptually, they are a filtered version since we report the values typically found in acompartmentreport but only for some specific points; specified in an additional file. The specs are described here.Neurodamus/neuron already handle most of this. However, coreneuron generates reports by itself (using libsonatareport) so we need to add support in coreneuron itself.
How
Qualitatively this is done in a few steps:
compartment_sets.jsonby itself. This is done by adding a few lines of binary values toreport.conf. A point is defined as a triplet: (gid,section_id,compartment_id) where:gidis the usual gid of a cellsection_idis the progressive id of the sections. Local of each cell and it depends on the stection types that are on the cell. Qualitatively, it uniquely defines a section on the cell and you can always pass for exampledend[16] <-> section_id.compartment_idI am not 100% sure of how these ids are really taken. However I see that I can:seg(compartment) withsoma[0](0.5).node_index()node. A node is an element in one of the basic neuron matricesMemb_list.nodeindiceswhere:Memb_list.nodeindices[i_node] = compartment_id.Memb_list.nodeindicesallows me to retrieve the node id to, finally, pick the correctdouble*necessary for libsoantareport to work. This lazy mapping is done infill_segment_id_2_node_id(.get_var(is in charge of calling this when necessary. The only thing left to manage this cache is to decide when to clear the cache (every new gid gets a new cache). Fortunately gids are progressive so this is already optimal.Changelog
nrnreport.hppCompartmentSetreport typeTargetTypeand split it inCompartmentsandSectionType: no need to encode/decode 2 enums into 1 in a non-critical part of the codeReportConfigurationto manageCompartmentSetreport and the removal ofTargetTypereport_configuration_parser.cppreport.confvector<int>fromreport.confwithfill_vec_intnrn_assert:compartmentreports cannot pick multiple variables for the same pointreport_handler.cppfill_segment_id_2_node_id. Moved out ofget_varto carify its purpose. In some particular cases it may be useful to have it as a stand-alone functioncheck_section_type: if theReportTypeis unknown we throw an error directly increate_reportintersection_gidsto create a list of ids to pick inreport.target.CompartmentSetcan easily have repeatedgids(not a thing before) which were discarded before. This made the previous list badly suited for a 3-vector id system as inCompartmentSet. It could have been done without this change but this was by far the simplest, cleanest, and most elegant solution. The change for the existing functions is minimal. From:to:
nodes_to_gidsandmap_gidswhich is, now, obsolete and inefficient. It requires a loop over all the nodes instead of picking only the good ones.all_compartmentsis not part of the configuration (in a nice enum:Compartments)report_handler.hppreport_handler.cpp. Faster compilation.test/external/CMakeLists.txt