Support H5 morphology container and individual H5 morphologies#57
Support H5 morphology container and individual H5 morphologies#57darshanmandge merged 18 commits intomainfrom
Conversation
…for Cell simualtion and SONATA circuit simulations
…ore adding build/
- Add test for _is_valid_morphology_path method covering regular files and H5 containers - Add tests for type2name and mksubset class methods - Improve test coverage for new H5 morphology features This addresses codecov coverage gaps in the H5 morphology implementation.
- Fix W293 linting errors in test files - Remove trailing whitespace from blank lines in docstrings - Ensure all linting checks pass
- Add test for corrupted H5 container exception handling - Add test for SectionName dataclass functionality - Improve coverage from 92.34% to 92.39% (above HEAD's 92.36%) - Cover exception paths in template.py and morphio_wrapper.py - All tests passing (25 H5-related tests) This brings the branch coverage above HEAD and ensures all new H5 morphology functionality is thoroughly tested.
- Add test to verify soma conversion functions work correctly - Improves test coverage for morphology wrapper - Tests morph_as_hoc() output for soma sections Note: Codecov shows 81.36% patch coverage because it measures coverage of only the NEW lines added in the PR (403 lines in morphio_wrapper.py). Overall project coverage is 92.39% which is above HEAD's 92.36%. Some uncovered lines are edge cases (specific soma types, exception paths) that require rare morphology scenarios to trigger.
- Add test for make_convex with non-monotonic data (covers line 97) - Add test for _to_sphere function (covers lines 150-158) - Add test for single_point_sphere_to_circular_contour (covers lines 164-167) - Improves morphio_wrapper.py coverage from 90.80% to 97.70% - Overall project coverage improved to 93% Remaining uncovered lines (234, 235, 245, 247) are exception paths for invalid soma types that require malformed morphologies to trigger.
- Remove trailing whitespace from blank lines - Add noqa comment for morphio import availability check - All linting checks now pass
- Add test for split_morphology_path error condition (empty path) - Add test for H5 container path with cell name parsing - Add tests for contourcenter, get_sides, contour2centroid functions - Add integration test for nested H5 path validation - Add integration test for split_morphology_path with nested directories
darshanmandge
force-pushed
the
h5_morphology_and_container
branch
from
6be7729 to
a0b62d7
Compare
- add get_target_cell_ids tests for population and non-population node_set paths - add morph_filepath tests for h5v1, asc fallback, and wrapped error branches - add emodel_path fallback tests for hoc model_template and failure path
- make h5 integration path-validation tests self-contained with temp H5 files - remove conditional branches that skipped coverage in morphology wrapper tests - mark script-only __main__ block as no-cover - keep assertions deterministic for container/nested path parsing
bluecellulab/cell/template.py
Outdated
|
|
||
| if h5_index >= 0: | ||
| # The container file is everything up to and including the .h5 part | ||
| container_path = '/'.join(parts[:h5_index + 1]) |
There was a problem hiding this comment.
Maybe using os.path.join might be safer for cross platform compatibility.
There was a problem hiding this comment.
Fixed. The updated get_cell implementation no longer manually concatenates paths with /. Instead:
- For H5 containers: uses
os.path.relpath(morph_filepath, candidate)to extract the cell name - For regular files: uses
os.path.dirname(morph_filepath)andos.path.basename(morph_filepath) - The HOC template receives
morph_dirandmorph_fnameseparately and performs its own concatenation (sprint(morph_path, "%s/%s", $s1, $s2)), which is consistent with how neurodamus passes arguments to HOC templates.
…and fix run_bluecellulab_simulation.py - Use os.path.dirname walk-up (neurodamus-style) instead of string splitting - Use os.path.isfile instead of os.path.exists to reject directories - Simplify morph_fname construction in get_cell (remove redundant branches) - Add comprehensive cross-platform path handling for H5 containers - Update examples/2-sonata-network/run_circuit_mpi/run_bluecellulab_simulation.py to align with the script in bluenaas used for running platform simualtions.
AurelienJaquier
left a comment
AurelienJaquier
left a comment
There was a problem hiding this comment.
I haven't had time to go through the notebooks yet.
I am putting here already the comments I have about the BCL code
| """ | ||
| POINTS = 101 | ||
|
|
||
| points = np.vstack((np.diff(xyz[:, [X, Y]], axis=0), xyz[0, [X, Y]])) |
There was a problem hiding this comment.
Not really sure why we vstack xyz[0, [X, Y]] here, if we drop the last item in the next line anyway
There was a problem hiding this comment.
np.diff produces N−1 row vectors (each row is point[i+1] − point[i]). Appending xyz[0, [X, Y]] as the N-th row closes the contour: it represents the displacement from the last point back to the first point. np.hstack((0,), norm(points, ...)) prepends a zero, making a length-N+1 array of perimeter distances. [:-1] then drops the last value — which is the full perimeter distance at the wrap-around point — leaving exactly N perimeter distances, one per original contour point. This matches the HOC in import3d_sec.hoc. I've added a comment in the code to explain this.
|
|
||
| # Walk up via os.path.dirname (same as neurodamus split_morphology_path) | ||
| candidate = morph_filepath | ||
| while not os.path.exists(candidate): |
There was a problem hiding this comment.
This logic is very similar to the one in
and in the get_cell() method.I think you should put this logic in a function and call it so that you don't have to duplicate it in several places
There was a problem hiding this comment.
Refactored get_cell to call split_morphology_path for the directory resolution. One subtlety: split_morphology_path uses os.path.splitext, which splits on the last dot and mangles cell names that contain dots (e.g. …Scale_x1.000_y0.950_z1.000_-_Clone_0). For the H5-container branch, morph_fname is therefore still computed via os.path.relpath to recover the full bare cell name before appending .h5 — this is documented in a comment.
| else: | ||
| # For node_sets without population filter, query all populations | ||
| ids = self._circuit.nodes.ids(node_set_def) | ||
| return {CellId(x.population, x.id) for x in ids} |
There was a problem hiding this comment.
Why is this line different from line 270?
There was a problem hiding this comment.
The two branches query SNAP in different ways:
- Line 270 (CellId(population, x)): NodePopulation.ids() returns plain int node IDs, so the population name is already known from the node-set definition.
- Line 274 (CellId(x.population, x.id)): Circuit.nodes.ids() (multi-population) returns CircuitNodeId objects that carry both .population and .id, so both fields are extracted from each element.
I've added comments to both branches in the code to make this explicit.
AurelienJaquier
left a comment
AurelienJaquier
left a comment
There was a problem hiding this comment.
thanks, looks good
| "id": "6f011909", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "This is an advanced example. Please refer to [singlecell.ipynb notebook](singlecell.ipynb) for a basic example.Morphology formats that can be run by BlueCelluLab are: [asc](https://morphio.readthedocs.io/en/latest/specification_neurolucida.html#id1), [swc](https://swc-specification.readthedocs.io/en/latest/swc.html), [H5](https://morphology-documentation.readthedocs.io/en/latest/h5v1.html#file-format) and [H5 container](https://morphology-documentation.readthedocs.io/en/latest/h5-containers.html).\n", |
There was a problem hiding this comment.
typo: space missing after the point in example.Morphology
3 participants
Description
Adds support for H5 morphology containers and individual H5 morphology files to BlueCelluLab, aligning the implementation with Neurodamus. It also fixes critical issues with node set resolution in multi-population circuits that were causing "Invalid range: 0-0" errors.
H5 Morphology Support
Core Implementation Changes
Bug Fixes
Examples and Tests
Dependencies
Added: morphio (required for H5 morphology support)