Skip to content

Commit aeed2ab

Browse files
zachcpclaude
andcommitted
fix(ferritin-core): 9 API correctness and performance fixes
- default_distance_range: return Option<(f32,f32)> instead of panicking on unknown atom pair - vdw_radius: remove duplicate Bondi-1964 table; Element::atomic_radius() is the single source - intern feature: delete dead-code intern.rs and feature flag - atom_order: cache in OnceLock<HashMap<&str, usize>> — eliminates per-call heap allocation - Segmentation::from_offsets: validate monotonicity to prevent silent corrupt state - BondOrder: add AromaticSingle/AromaticDouble variants (CCD codes 5/6 were silently mislabeled) - OrderedSet::iter: replace Box<dyn Iterator> with zero-alloc OrderedSetIter enum - AminoAcid enum: replace three parallel match tables with a typed enum + thin wrappers - AtomsTable.element: change Vec<String> to Vec<Element> — fixes silent Carbon fallback on round-trip Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent cfe1aae commit aeed2ab

21 files changed

Lines changed: 248 additions & 297 deletions

File tree

.beads/interactions.jsonl

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,13 @@
7979
{"id":"int-2d686c6a90c0dc2c2b0b90f6113830a6","kind":"field_change","created_at":"2026-06-29T11:45:00.208583Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-pvg","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Bevy 0.19 auto-enables VERTEX_COLORS shader when mesh has ATTRIBUTE_COLOR. Confirmed vertex colors already set in mesh; build+tests pass."}}
8080
{"id":"int-2c133f49a4748c2f22624fb9adfa3755","kind":"field_change","created_at":"2026-06-29T11:45:00.799531Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-hn9","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Rewrote render_cartoon() to use generate_cartoon_mesh() with per-curve-point SS assignment. Added vertex colors (salmon/periwinkle/green) to cartoon mesh. Removed broken generate_alpha_helix_mesh / generate_beta_sheet_mesh."}}
8181
{"id":"int-628cc6166189d5ab997b6c8c15c5fe66","kind":"field_change","created_at":"2026-06-29T11:45:01.383696Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-jbs","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Added OrbitCamera component + orbit_camera system to interactive_viewer.rs. Left-drag=orbit, right-drag=pan, scroll=zoom."}}
82+
{"id":"int-104f7be4c5e009c36095058e933c76d5","kind":"field_change","created_at":"2026-06-29T12:21:53.97428Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-4tl","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"All non-deferred phases complete: P-1 through P5a landed (data/ primitives, Model/Hierarchy/Conformation, Trajectory, IO model-splitting, AtomCollection adapter, Unit view). Deferred items filed as ferritin-229 (P5b: SymmetryOperator) and ferritin-h04 (P5c: selection DSL). Dead-code cleanup and atom37/AA-index consolidation into ferritin-core::info done on feat/ferritin-4tl-resume branch."}}
83+
{"id":"int-24dc181a89e17c90f5a220e3207601c0","kind":"field_change","created_at":"2026-06-29T12:32:45.300326Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-2l3","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
84+
{"id":"int-d8dec6f9ef080285d72116751f4c6b25","kind":"field_change","created_at":"2026-06-29T12:32:45.898357Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-ynd","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
85+
{"id":"int-a95e52a60f1455154d910213b7439f0b","kind":"field_change","created_at":"2026-06-29T12:32:46.483357Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-0ao","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
86+
{"id":"int-9ebdecd1092aeabf68eeeea545cf771d","kind":"field_change","created_at":"2026-06-30T01:41:10.346296Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-nd0","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
87+
{"id":"int-811db0f32604800bad59179531e2fe40","kind":"field_change","created_at":"2026-06-30T01:41:11.372553Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-d0k","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
88+
{"id":"int-b5de508208c77bda02883a90b099db1b","kind":"field_change","created_at":"2026-06-30T01:41:12.195374Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-70w","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
89+
{"id":"int-dbcd26bd3705ca83f13be9ba1eac2220","kind":"field_change","created_at":"2026-06-30T01:43:46.503278Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-4iv","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
90+
{"id":"int-291e15638b84b558373f9b674ec0f2e9","kind":"field_change","created_at":"2026-06-30T01:43:47.220266Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-0eg","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}
91+
{"id":"int-a854d1eb6633ae97335602f7eea255b1","kind":"field_change","created_at":"2026-06-30T01:49:00.285368Z","actor":"Zachary Charlop-Powers","issue_id":"ferritin-h6y","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}}

crates/ferritin-core/Cargo.toml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,6 @@ authors.workspace = true
66
license.workspace = true
77
description.workspace = true
88

9-
[features]
10-
default = []
11-
intern = []
12-
139
[dependencies]
1410
anyhow.workspace = true
1511
itertools.workspace = true

crates/ferritin-core/src/atomcollection.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ impl AtomCollection {
367367

368368
let atoms = AtomsTable {
369369
atom_name: self.atom_names.clone(),
370-
element: self.elements.iter().map(|e| e.symbol().to_string()).collect(),
370+
element: self.elements.clone(),
371371
alt_loc: vec![None; n_atoms],
372372
formal_charge: vec![None; n_atoms],
373373
};
@@ -492,12 +492,7 @@ impl From<&Model> for AtomCollection {
492492
chain_ids.push(hierarchy.chains.auth_asym_id[chain_idx].clone());
493493
}
494494

495-
let elements: Vec<Element> = hierarchy
496-
.atoms
497-
.element
498-
.iter()
499-
.map(|s| Element::from_symbol(s).unwrap_or(Element::C))
500-
.collect();
495+
let elements = hierarchy.atoms.element.clone();
501496

502497
let atom_names = hierarchy.atoms.atom_name.clone();
503498

crates/ferritin-core/src/bonds.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ pub enum BondOrder {
4545
Double = 2,
4646
Triple = 3,
4747
Quadruple = 4,
48+
AromaticSingle = 5,
49+
AromaticDouble = 6,
4850
}
4951

5052
impl From<i32> for BondOrder {
@@ -54,7 +56,10 @@ impl From<i32> for BondOrder {
5456
1 => BondOrder::Single,
5557
2 => BondOrder::Double,
5658
3 => BondOrder::Triple,
57-
_ => BondOrder::Quadruple,
59+
4 => BondOrder::Quadruple,
60+
5 => BondOrder::AromaticSingle,
61+
6 => BondOrder::AromaticDouble,
62+
_ => BondOrder::Unset,
5863
}
5964
}
6065
}

crates/ferritin-core/src/data/intern.rs

Lines changed: 0 additions & 116 deletions
This file was deleted.
Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
1-
pub mod intern;
21
pub mod ordered_set;
32
pub mod segmentation;
43

54
pub use ordered_set::OrderedSet;
65
pub use segmentation::Segmentation;
7-
8-
#[cfg(feature = "intern")]
9-
pub use intern::{InternedId, Interner};

crates/ferritin-core/src/data/ordered_set.rs

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,22 @@
55
66
use std::sync::Arc;
77

8+
/// Allocation-free iterator over an [`OrderedSet`].
9+
pub enum OrderedSetIter<'a> {
10+
Interval(std::ops::Range<u32>),
11+
Sorted(std::iter::Copied<std::slice::Iter<'a, u32>>),
12+
}
13+
14+
impl<'a> Iterator for OrderedSetIter<'a> {
15+
type Item = u32;
16+
fn next(&mut self) -> Option<u32> {
17+
match self {
18+
OrderedSetIter::Interval(r) => r.next(),
19+
OrderedSetIter::Sorted(it) => it.next(),
20+
}
21+
}
22+
}
23+
824
/// Ordered set of indices — either a contiguous interval or sorted array.
925
///
1026
/// The `Interval` variant provides O(1) membership tests for contiguous ranges.
@@ -77,19 +93,10 @@ impl OrderedSet {
7793
}
7894

7995
/// Iterate over the elements of this set in ascending order.
80-
pub fn iter(&self) -> impl Iterator<Item = u32> + '_ {
96+
pub fn iter(&self) -> OrderedSetIter<'_> {
8197
match self {
82-
OrderedSet::Interval { start, end } => {
83-
// Use a box to unify the two iterator types
84-
let iter: Box<dyn Iterator<Item = u32> + '_> =
85-
Box::new(*start..*end);
86-
iter
87-
}
88-
OrderedSet::Sorted(v) => {
89-
let iter: Box<dyn Iterator<Item = u32> + '_> =
90-
Box::new(v.iter().copied());
91-
iter
92-
}
98+
OrderedSet::Interval { start, end } => OrderedSetIter::Interval(*start..*end),
99+
OrderedSet::Sorted(v) => OrderedSetIter::Sorted(v.iter().copied()),
93100
}
94101
}
95102

crates/ferritin-core/src/data/segmentation.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ impl Segmentation {
2222
/// Panics if `offsets` is empty (needs at least the sentinel `[0]`).
2323
pub fn from_offsets(offsets: Vec<u32>) -> Self {
2424
assert!(!offsets.is_empty(), "offsets must have at least one element");
25+
for window in offsets.windows(2) {
26+
assert!(
27+
window[0] <= window[1],
28+
"offsets must be monotonically non-decreasing; found {} > {}",
29+
window[0],
30+
window[1]
31+
);
32+
}
2533
Self { offsets }
2634
}
2735

@@ -176,4 +184,10 @@ mod tests {
176184
let ranges: Vec<Range<usize>> = seg.iter().collect();
177185
assert_eq!(ranges, vec![0..3, 3..5, 5..6]);
178186
}
187+
188+
#[test]
189+
#[should_panic(expected = "monotonically non-decreasing")]
190+
fn test_segmentation_from_offsets_non_monotonic_panics() {
191+
Segmentation::from_offsets(vec![0, 10, 5, 15]);
192+
}
179193
}

0 commit comments

Comments
 (0)