Skip to content

Commit

Permalink
[REP-91] Avoid raising exception when point process is missing in a s…
Browse files Browse the repository at this point in the history
…ection (#202)

## Context
Simulation was crashing when a point process was missing in a section in
the context of summation reports.
Refactor the code to improve readability and avoid raising the exception
to maintain consistency with CoreNEURON behavior

## Review
* [x] PR description is complete
* [x] Coding style (imports, function length, New functions, classes or
files) are good
* [x] Unit/Scientific test added
* [ ] Updated Readme, in-code, developer documentation
  • Loading branch information
jorblancoa authored Oct 30, 2024
1 parent 629b989 commit 41eb662
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 41 deletions.
82 changes: 41 additions & 41 deletions neurodamus/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def get_section_index(cell, section):

class Report:
INTRINSIC_CURRENTS = {"i_membrane", "i_membrane_", "ina", "ica", "ik", "i_pas", "i_cap"}
CURRENT_INJECTING_PROCESSES = {"SEClamp", "IClamp"}

def __init__(self, report_name, report_type, variable_name, unit, format, dt, start_time,
end_time, output_dir, scaling_option=None, use_coreneuron=False):
Expand Down Expand Up @@ -97,8 +98,7 @@ def add_summation_report(self, cell_obj, point, collapsed, vgid,
if not collapsed:
alu_helper = self.setup_alu_for_summation(x, collapsed)

self.handle_intrinsic_currents(section, x, alu_helper, variable_names)
self.handle_point_processes(section, x, alu_helper, variable_names)
self.handle_currents_and_point_processes(section, x, alu_helper, variable_names)

if not collapsed:
section_index = get_section_index(cell_obj, section)
Expand Down Expand Up @@ -141,46 +141,46 @@ def add_synapse_report(self, cell_obj, point, vgid, pop_name="default", pop_offs
except AttributeError:
raise AttributeError(f"Variable '{variable}' not found at '{synapse.hname()}'.")

def handle_point_processes(self, section, x, alu_helper, variable_names):
"""Handle point processes for summation report."""
scalar = 1
for mechanism, variable in variable_names:
point_processes = self.get_point_processes(section, mechanism)
for point_process in point_processes:
if self.is_point_process_at_location(point_process, section, x):
if "SEClamp" in point_process.hname() or "IClamp" in point_process.hname():
scalar = -1
try:
var_ref = getattr(point_process, '_ref_' + variable)
alu_helper.addvar(var_ref, scalar)
except AttributeError:
err = f"Variable '{variable}' not found at '{point_process.hname()}'."
raise AttributeError(err)
Nd.pop_section()

def handle_intrinsic_currents(self, section, x, alu_helper, variable_names):
"""Handle intrinsic currents for summation report."""
# Intrinsic currents processing
def handle_currents_and_point_processes(self, section, x, alu_helper, variable_names):
"""Handle both intrinsic currents and point processes for summation report."""
area_at_x = section(x).area()
scalar = 1
for mechanism, _ in variable_names:
if self.get_point_processes(section, mechanism):
# Ignore point processes, they are handled separately
continue
if mechanism != "i_membrane" and self.scaling_mode == 1: # Area scaling
# Need to convert distributed current sources units.
# NEURON stores/reports them as mA/cm^2; we want nA.
scalar = area_at_x / 100.0
if area_at_x and mechanism not in {"SEClamp", "IClamp"}:
mechanism = self.enable_fast_imem(mechanism)
try:
var_ref = getattr(section(x), '_ref_' + mechanism)
alu_helper.addvar(var_ref, scalar)
except AttributeError:
if mechanism in Report.INTRINSIC_CURRENTS:
logging.warning(f"Current '{mechanism}' does not exist at {section(x)}")
else:
raise AttributeError(f"Mechanism '{mechanism}' not found.")
for mechanism, variable in variable_names:
self.process_mechanism(section, x, alu_helper, mechanism, variable, area_at_x)

def process_mechanism(self, section, x, alu_helper, mechanism, variable, area_at_x):
"""Process a single mechanism, whether it's an intrinsic current or a point process."""
point_processes = self.get_point_processes(section, mechanism)
if point_processes:
self.handle_point_processes(section, x, alu_helper, point_processes, variable)
elif area_at_x:
self.handle_intrinsic_current(section, x, alu_helper, mechanism, area_at_x)
else:
logging.warning(f"Skipping intrinsic current '{mechanism}' at a location with area = 0")

def handle_point_processes(self, section, x, alu_helper, point_processes, variable):
"""Handle point processes for a given mechanism."""
for point_process in point_processes:
if self.is_point_process_at_location(point_process, section, x):
is_inverted = any(proc in point_process.hname()
for proc in self.CURRENT_INJECTING_PROCESSES)
scalar = -1 if is_inverted else 1
self.add_variable_to_alu(alu_helper, point_process, variable, scalar)
Nd.pop_section()

def handle_intrinsic_current(self, section, x, alu_helper, mechanism, area_at_x):
"""Handle an intrinsic current mechanism."""
scalar = area_at_x / 100.0 if mechanism != "i_membrane" and self.scaling_mode == 1 else 1
mechanism = self.enable_fast_imem(mechanism)
self.add_variable_to_alu(alu_helper, section(x), mechanism, scalar)

def add_variable_to_alu(self, alu_helper, obj, variable, scalar):
"""Add a variable to the ALU helper with error handling."""
try:
var_ref = getattr(obj, '_ref_' + variable)
alu_helper.addvar(var_ref, scalar)
except AttributeError:
if variable in self.INTRINSIC_CURRENTS:
logging.warning(f"Current '{variable}' does not exist at {obj}")

def setup_alu_for_summation(self, alu_x, collapsed):
"""Setup ALU helper for summation."""
Expand Down
10 changes: 10 additions & 0 deletions tests/integration-e2e/test_reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,16 @@ def _create_reports_config(original_config_path: Path, tmp_path: Path) -> tuple[
"start_time": 0.0,
"end_time": 40.0
}
# Added to verify no exception is raised when point process is not present in a section
config["reports"]["summation_ProbGABAAB"] = {
"type": "summation",
"cells": "Mosaic",
"variable_name": "ProbGABAAB_EMS.i",
"sections": "all",
"dt": 0.1,
"start_time": 0.0,
"end_time": 40.0
}

# Write the modified configuration to a temporary file
temp_config_path = tmp_path / "reports_config.json"
Expand Down

0 comments on commit 41eb662

Please sign in to comment.