Skip to content

Commit 584afb7

Browse files
committed
refactor(smithereens): move graph to adjacency list
1 parent 9a1a3f9 commit 584afb7

File tree

1 file changed

+87
-42
lines changed

1 file changed

+87
-42
lines changed

crates/smithereens/src/lib.rs

+87-42
Original file line numberDiff line numberDiff line change
@@ -21,67 +21,90 @@ impl Display for Terminal<'_> {
2121
}
2222
}
2323

24-
#[derive(Clone, Debug)]
24+
#[derive(Clone, Debug, Default)]
2525
struct Residue<'p> {
26-
id: ResidueId,
2726
terminals: Vec<Terminal<'p>>,
28-
}
29-
30-
impl<'p> Residue<'p> {
31-
const fn new(id: ResidueId) -> Self {
32-
let terminals = Vec::new();
33-
Self { id, terminals }
34-
}
27+
bonds: Vec<Bond<'p>>,
3528
}
3629

3730
#[derive(Copy, Clone, Debug)]
3831
struct Bond<'p> {
39-
donor: ResidueId,
40-
abbr: BondAbbr<'p>,
41-
acceptor: ResidueId,
32+
// FIXME: The naming `end` doesn't fit well with the `Terminal` here...
33+
end: Terminal<'p>,
34+
target: usize,
4235
}
4336

4437
impl<'p> Bond<'p> {
45-
// FIXME: Should this be a From impl instead / as well?
46-
const fn new(bond_info: &BondInfo<'_, 'p>) -> Self {
47-
let &BondInfo(ResidueGroup(donor, _), ref bond, ResidueGroup(acceptor, _)) = bond_info;
48-
let abbr = bond.abbr();
38+
const fn donating_to(abbr: BondAbbr<'p>, acceptor: usize) -> Self {
39+
let end = Terminal::Donor(abbr);
4940
Self {
50-
donor,
51-
abbr,
52-
acceptor,
41+
end,
42+
target: acceptor,
5343
}
5444
}
45+
46+
const fn accepting_from(abbr: BondAbbr<'p>, donor: usize) -> Self {
47+
let end = Terminal::Acceptor(abbr);
48+
Self { end, target: donor }
49+
}
5550
}
5651

52+
// PERF: Not sold on this "tombstone" approach with `Option` — might be better to re-index the sub-graphs so their
53+
// vectors can be shrunk?
5754
#[derive(Clone, Debug)]
58-
struct Fragment<'p> {
59-
depth: u32,
60-
residues: Vec<Residue<'p>>,
61-
bonds: Vec<Bond<'p>>,
62-
}
55+
struct Fragment<'p>(Vec<Option<Residue<'p>>>);
6356

6457
// FIXME: This was written way too quickly... Take a look back at this...
58+
// FIXME: Also maybe should die, as it's only useful for debugging?
6559
impl Display for Fragment<'_> {
6660
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
6761
writeln!(f, "digraph {{")?;
68-
for Residue { id, terminals } in &self.residues {
62+
for (id, residue) in self.0.iter().enumerate() {
63+
let Some(Residue { terminals, bonds }) = residue else {
64+
continue;
65+
};
66+
6967
let terminals = terminals.iter().map(ToString::to_string).join(", ");
7068
writeln!(f, r#" {id} [label="{id}\n[{terminals}]"]"#)?;
71-
}
72-
for Bond {
73-
donor,
74-
abbr,
75-
acceptor,
76-
} in &self.bonds
77-
{
78-
writeln!(f, r#" {donor} -> {acceptor} [label="{abbr}"]"#)?;
69+
70+
// NOTE: The `.filter()` here prevents the double-printing of edges
71+
for Bond { end, target } in bonds.iter().filter(|b| id < b.target) {
72+
match end {
73+
Terminal::Donor(abbr) => {
74+
writeln!(f, r#" {id} -> {target} [label="{abbr}"]"#)?;
75+
}
76+
Terminal::Acceptor(abbr) => {
77+
writeln!(f, r#" {target} -> {id} [label="{abbr}"]"#)?;
78+
}
79+
}
80+
}
7981
}
8082
writeln!(f, "}}")?;
8183
Ok(())
8284
}
8385
}
8486

87+
#[derive(Clone, Debug)]
88+
struct NodeMapping(Vec<ResidueId>);
89+
90+
impl NodeMapping {
91+
fn new(polymer: &Polymer) -> Self {
92+
// FIXME: Needs a second look?
93+
// PERF: Is there some way to collect and sort at the same time? Would that even be faster?
94+
let mut node_mapping: Vec<_> = polymer.residue_ids().collect();
95+
node_mapping.sort_unstable();
96+
Self(node_mapping)
97+
}
98+
99+
// NOTE: Keeping `id` as a ref, since it needs to be one for `binary_search()` and because it comes from
100+
// `bond_refs()` to begin with!
101+
#[allow(clippy::trivially_copy_pass_by_ref)]
102+
fn index(&self, id: &ResidueId) -> usize {
103+
// SAFETY: Panics if the `id` isn't found
104+
self.0.binary_search(id).unwrap()
105+
}
106+
}
107+
85108
// DESIGN: This `Sized` bound could be moved to the method-level with `where Self: Sized`, which would allow this trait
86109
// to be implemented for unsized types, just without those methods, but I don't think this trait makes much sense
87110
// without a `.fragment()` method (which currently requires a `Self: Sized`), so I've elected to make the whole trait
@@ -97,23 +120,45 @@ pub trait Dissociable: Sized {
97120
}
98121
// FIXME: Only for testing! Remove!
99122
fn dbg_fragment(&self) {
100-
eprintln!("{}", Fragment::new(self.polymer()));
123+
let polymer = self.polymer();
124+
let node_mapping = NodeMapping::new(polymer);
125+
let fragment = Fragment::new(&node_mapping, polymer);
126+
eprintln!("{fragment}");
101127
}
102128
}
103129

104130
impl<'p> Fragment<'p> {
105-
fn new<'r>(polymer: &'r Polymer<'_, 'p>) -> Self {
106-
let depth = 0;
107-
let residues: Vec<Residue<'p>> = polymer.residue_ids().map(Residue::new).collect();
108-
let bonds: Vec<Bond<'p>> = polymer.bond_refs().map(Bond::new).collect();
109-
Self {
110-
depth,
111-
residues,
112-
bonds,
131+
// FIXME: This currently assumes that the `polymer` provided consists of a single connected component. If this is
132+
// not the case, I should report an error to the user!
133+
fn new(node_mapping: &NodeMapping, polymer: &Polymer<'_, 'p>) -> Self {
134+
let mut residues = vec![Some(Residue::default()); node_mapping.0.len()];
135+
136+
for BondInfo(ResidueGroup(donor, _), bond, ResidueGroup(acceptor, _)) in polymer.bond_refs()
137+
{
138+
let abbr = bond.abbr();
139+
let donor = node_mapping.index(donor);
140+
let acceptor = node_mapping.index(acceptor);
141+
142+
let mut push_bond =
143+
// FIXME: Shocking failure of type inference here with that `usize`...
144+
|residue: usize, bond| residues[residue].as_mut().unwrap().bonds.push(bond);
145+
146+
push_bond(donor, Bond::donating_to(abbr, acceptor));
147+
push_bond(acceptor, Bond::accepting_from(abbr, donor));
113148
}
149+
150+
Self(residues)
114151
}
115152
}
116153

154+
// impl<'p> Index<usize> for Fragment<'p> {
155+
// type Output = Residue<'p>;
156+
157+
// fn index(&self, index: usize) -> &Self::Output {
158+
// self.0[index].as_ref().unwrap()
159+
// }
160+
// }
161+
117162
// SEE NOTES FROM APRIL 8TH!
118163
// use DashMap or quick-cache for a global fragment cache
119164

0 commit comments

Comments
 (0)