Skip to content

fixing mapping of syn loc from BMTK to Arbor#4

Merged
thorstenhater merged 3 commits into
thorstenhater:mainfrom
beaherrera:fix-syn-locs
Sep 13, 2025
Merged

fixing mapping of syn loc from BMTK to Arbor#4
thorstenhater merged 3 commits into
thorstenhater:mainfrom
beaherrera:fix-syn-locs

Conversation

@beaherrera

Copy link
Copy Markdown
Collaborator

Modifications:

  • Added export of soma location and rotation angles as part of the cell's metadata.
  • Updated main.py to move and rotate Arbor's morph to match BMTK coordinates.

- moved and rotated Arbor's morph to match BMTK coordinates
Comment thread src/gen.rs Outdated
ModelType::Point { .. } => String::from("point"),
ModelType::Virtual { .. } => String::from("virtual"),
};
let (pos_x, pos_y, pos_z) = node.position;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're unpacking a tuple here, just to repack the components a few lines down.

Comment thread src/sim.rs Outdated
}

// Extract position coordinates (x, y, z)
let pos_x = if let Some(xs) = group.custom.get("x") {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be written bit more concisely by using a closure:

let get_coord = |c: &str| -> f64 { 
       if let Some(cs) = group.custom.get(c) {
            cs[group_index]
        } else if let Some(Attribute::Float(c)) = node_type.attributes().get(c) {
            *c
        } else {
            0.0 // default position
        }
};

and then using that for the three axes.

Comment thread src/sim.rs
0.0 // default position
};

// Extract rotation angles

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same idea here.

Comment thread data/main.py Outdated
for x, y, z, synapse, params, tag in self.gid_to_syn[gid]:
loc, _ = pwl.closest(x, y, z)
arborxyz = pwl.at(loc)
print(f"Placing synapse {synapse} at {loc}: {x},{y},{z} vs. {arborxyz}")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to keep this for now or is it purely debug info?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's purely debug info. I'll remove it, and we can add it locally when needed.

@thorstenhater

Copy link
Copy Markdown
Owner

Hey @beaherrera,

thanks, I left a few mostly stylistic comments but otherwise good to merge.
Please re-run cargo fmt again.

@beaherrera

Copy link
Copy Markdown
Collaborator Author

Thank you, @thorstenhater!
I just made the changes.

@thorstenhater thorstenhater left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you 🎉

@thorstenhater thorstenhater merged commit 624acf0 into thorstenhater:main Sep 13, 2025
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants