fix duplicate bonds in max_dist and nearest_neighbor functions

causing unnecessary rendering work from coinciding cylinder
add methyl molecule

.pre-commit-config.yaml

@@ -26,10 +26,10 @@ repos:
           - prettier-plugin-svelte
           - svelte
         stages: [commit]
-        exclude: ^changelog.md|.*/structures/.*\.json|.py$
+        exclude: ^changelog.md|.*/(structures|molecules)/.*\.json|.py$
 
   - repo: https://github.com/pre-commit/mirrors-eslint
-    rev: v8.44.0
+    rev: v8.46.0
     hooks:
       - id: eslint
         types: [file]

package.json

@@ -36,13 +36,13 @@
     "d3-scale-chromatic": "^3.0.0",
     "d3-shape": "^3.2.0",
     "highlight.js": "^11.8.0",
-    "svelte": "4.1.0",
+    "svelte": "4.1.1",
     "svelte-multiselect": "^10.1.0",
-    "three": "^0.154.0"
+    "three": "^0.155.0"
   },
   "devDependencies": {
-    "@playwright/test": "^1.36.1",
-    "@sveltejs/adapter-static": "2.0.2",
+    "@playwright/test": "^1.36.2",
+    "@sveltejs/adapter-static": "2.0.3",
     "@sveltejs/package": "^2.2.0",
     "@types/d3-array": "^3.0.5",
     "@types/d3-color": "^3.1.0",
@@ -51,29 +51,29 @@
     "@types/d3-scale-chromatic": "^3.0.0",
     "@types/d3-shape": "^3.1.1",
     "@types/three": "^0.154.0",
-    "@typescript-eslint/eslint-plugin": "^6.1.0",
-    "@typescript-eslint/parser": "^6.1.0",
+    "@typescript-eslint/eslint-plugin": "^6.2.0",
+    "@typescript-eslint/parser": "^6.2.0",
     "@vitest/coverage-c8": "^0.33.0",
-    "eslint": "^8.45.0",
-    "eslint-plugin-svelte": "^2.32.2",
+    "eslint": "^8.46.0",
+    "eslint-plugin-svelte": "^2.32.4",
     "hastscript": "^7.2.0",
     "jsdom": "^22.1.0",
     "mdsvex": "^0.11.0",
     "mdsvexamples": "^0.3.3",
     "prettier": "^3.0.0",
-    "prettier-plugin-svelte": "^3.0.0",
+    "prettier-plugin-svelte": "^3.0.3",
     "rehype-autolink-headings": "^6.1.1",
     "rehype-katex-svelte": "^1.2.0",
     "rehype-slug": "^5.1.0",
     "remark-math": "3.0.0",
-    "sharp": "^0.32.3",
+    "sharp": "^0.32.4",
     "svelte-check": "^3.4.6",
     "svelte-preprocess": "^5.0.4",
     "svelte-toc": "^0.5.5",
     "svelte-zoo": "^0.4.9",
     "svelte2tsx": "^0.6.19",
     "typescript": "5.1.6",
-    "vite": "^4.4.4",
+    "vite": "^4.4.7",
     "vitest": "^0.33.0"
   },
   "keywords": [

src/lib/structure/bonding.ts

@@ -2,68 +2,65 @@ import type { BondPair, PymatgenStructure } from '$lib'
 import { euclidean_dist } from '$lib'
 
 // TODO add unit tests for these functions
-
-// finds all pairs of atoms within the max_bond_dist cutoff
 export function max_dist(
   structure: PymatgenStructure,
-  { max_bond_dist = 3, min_bond_dist = 0.1 } = {}, // in Angstroms
+  { max_bond_dist = 3, min_bond_dist = 0.4 } = {}, // in Angstroms
 ): BondPair[] {
+  // finds all pairs of atoms within the max_bond_dist cutoff
   const bonds: BondPair[] = []
+  const bond_set: Set<string> = new Set()
 
   for (let idx = 0; idx < structure.sites.length; idx++) {
     const { xyz } = structure.sites[idx]
 
-    // Only consider pairs that haven't been considered before, and avoid self-bonds
     for (let idx_2 = idx + 1; idx_2 < structure.sites.length; idx_2++) {
       const { xyz: xyz_2 } = structure.sites[idx_2]
 
       const dist = euclidean_dist(xyz, xyz_2)
       if (dist < max_bond_dist && dist > min_bond_dist) {
-        bonds.push([xyz, xyz_2, idx, idx_2, dist])
+        const bond_key = [xyz, xyz_2].sort().toString()
+        if (!bond_set.has(bond_key)) {
+          bond_set.add(bond_key)
+          bonds.push([xyz, xyz_2, idx, idx_2, dist])
+        }
       }
     }
   }
   return bonds
 }
 
-// finds bonds to sites less than scaling_factor farther away than the nearest neighbor
 export function nearest_neighbor(
   structure: PymatgenStructure,
-  { scaling_factor = 1.3, min_bond_dist = 0.1 } = {}, // in Angstroms
+  { scaling_factor = 1.2, min_bond_dist = 0.1 } = {}, // in Angstroms
 ): BondPair[] {
+  // finds bonds to sites less than scaling_factor farther away than the nearest neighbor
   const num_sites = structure.sites.length
-  // Initialize distance matrix with Infinity
-  const dists: number[][] = Array(num_sites)
-    .fill(null)
-    .map(() => Array(num_sites).fill(Infinity))
+  const bonds: BondPair[] = []
+  const bond_set: Set<string> = new Set()
 
-  // Calculate all pairwise distances once
-  for (let idx = 0; idx < num_sites; idx++) {
-    const { xyz } = structure.sites[idx]
-    for (let idx_2 = idx + 1; idx_2 < num_sites; idx_2++) {
-      const dist = euclidean_dist(xyz, structure.sites[idx_2].xyz)
-      dists[idx][idx_2] = dist
-      dists[idx_2][idx] = dist
+  for (let i = 0; i < num_sites; i++) {
+    const { xyz: xyz1 } = structure.sites[i]
+    let min_dist = Infinity
+
+    for (let j = i + 1; j < num_sites; j++) {
+      const { xyz: xyz2 } = structure.sites[j]
+      const dist = euclidean_dist(xyz1, xyz2)
+
+      if (dist > min_bond_dist && dist < min_dist) {
+        min_dist = dist
+      }
     }
-  }
 
-  const bonds: BondPair[] = []
-  for (let idx = 0; idx < num_sites; idx++) {
-    // Find the minimum distance (ignoring zero)
-    const min_dist = Math.min(
-      ...dists[idx].filter((dist) => dist > min_bond_dist),
-    )
+    for (let j = i + 1; j < num_sites; j++) {
+      const { xyz: xyz2 } = structure.sites[j]
+      const dist = euclidean_dist(xyz1, xyz2)
 
-    for (let idx_2 = idx + 1; idx_2 < num_sites; idx_2++) {
-      const dist = dists[idx][idx_2]
       if (dist <= min_dist * scaling_factor) {
-        bonds.push([
-          structure.sites[idx].xyz,
-          structure.sites[idx_2].xyz,
-          idx,
-          idx_2,
-          dist,
-        ])
+        const bond_key = [xyz1, xyz2].sort().toString()
+        if (!bond_set.has(bond_key)) {
+          bond_set.add(bond_key)
+          bonds.push([xyz1, xyz2, i, j, dist])
+        }
       }
     }
   }

src/routes/(demos)/molecule/+page.md

@@ -30,7 +30,7 @@
 </form>
 
 <h3 align='center'>{name}</h3>
-<Structure structure={molecule} />
+<Structure structure={molecule} bonding_strategy="max_dist" bonding_options={{max_bond_dist: 2}} />
 
 <style>
   form {

src/site/molecules/methyl.json

@@ -0,0 +1 @@
+{"@module": "pymatgen.core.structure", "@class": "Molecule", "charge": 0.0, "spin_multiplicity": 1, "sites": [{"name": "C", "species": [{"element": "C", "occu": 1}], "xyz": [0.0, 0.0, 0.0], "properties": {}, "label": "C"}, {"name": "H", "species": [{"element": "H", "occu": 1}], "xyz": [-0.51336, 0.889165, -0.363], "properties": {}, "label": "H"}, {"name": "C", "species": [{"element": "C", "occu": 1}], "xyz": [0.0, 0.0, 1.54], "properties": {}, "label": "C"}, {"name": "H", "species": [{"element": "H", "occu": 1}], "xyz": [0.51336, 0.889165, 1.903], "properties": {}, "label": "H"}, {"name": "H", "species": [{"element": "H", "occu": 1}], "xyz": [0.51336, -0.889165, 1.903], "properties": {}, "label": "H"}, {"name": "F", "species": [{"element": "F", "occu": 1}], "xyz": [-0.6363964562680897, -1.1022702490213807, -0.44999983174637], "properties": {}, "label": "F"}, {"name": "O", "species": [{"element": "O", "occu": 1}], "xyz": [1.3482169227095189, -2.220446049250313e-16, -0.47666668576655874], "properties": {}, "label": "O"}, {"name": "H", "species": [{"element": "H", "occu": 1}], "xyz": [1.3339599150845083, -0.2943909787153237, -1.3972349865751483], "properties": {}, "label": "H"}, {"name": "C", "species": [{"element": "C", "occu": 1}], "xyz": [-1.4519259167640972, 0.0, 2.053333353902448], "properties": {}, "label": "C"}, {"name": "H", "species": [{"element": "H", "occu": 1}], "xyz": [-1.7913371700336262, -1.01, 2.1733333587108126], "properties": {}, "label": "H"}, {"name": "H", "species": [{"element": "H", "occu": 1}], "xyz": [-2.0813371816538404, 0.5099999999999999, 1.3530894966427836], "properties": {}, "label": "H"}, {"name": "H", "species": [{"element": "H", "occu": 1}], "xyz": [-1.501337158413412, 0.5099999999999999, 2.9935772207788416], "properties": {}, "label": "H"}]}

tests/unit/Structure.test.ts

@@ -8,14 +8,17 @@ const structure = structures[0]
 
 describe(`Structure`, () => {
   test(`open control panel when clicking toggle button`, async () => {
-    new Structure({ target: document.body, props: { structure } })
+    const struct = new Structure({
+      target: document.body,
+      props: { structure },
+    })
 
     const dialog = doc_query<HTMLDialogElement>(`dialog`)
     expect(dialog.open).toBe(false)
     doc_query(`button.controls-toggle`).click()
     await tick()
 
-    expect(dialog.open).toBe(true)
+    expect(struct.controls_open).toBe(true)
   })
 
   test(`JSON file download when clicking download button`, async () => {

tests/unit/structure-utils.test.ts

@@ -109,8 +109,10 @@ test.each(structures)(`symmetrize_structure`, async (structure) => {
 
 test.each(structures)(`get_center_of_mass for $id`, async (struct) => {
   const center = module.get_center_of_mass(struct)
+  const expected = ref_data[struct.id]?.center_of_mass
+  if (!expected) return
   expect(
     center.map((val) => Math.round(val * 1e3) / 1e3),
     struct.id,
-  ).toEqual(ref_data[struct.id].center_of_mass)
+  ).toEqual(expected)
 })