From 14027e36af155b40f332f5768c7162ec919b0809 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sat, 29 Jul 2023 15:15:11 -0700 Subject: [PATCH] fix duplicate bonds in max_dist and nearest_neighbor functions causing unnecessary rendering work from coinciding cylinder add methyl molecule --- .pre-commit-config.yaml | 4 +- package.json | 22 +++++----- src/lib/structure/bonding.ts | 65 +++++++++++++--------------- src/routes/(demos)/molecule/+page.md | 2 +- src/site/molecules/methyl.json | 1 + tests/unit/Structure.test.ts | 7 ++- tests/unit/structure-utils.test.ts | 4 +- 7 files changed, 54 insertions(+), 51 deletions(-) create mode 100644 src/site/molecules/methyl.json diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4ab2473..9b25b40 100644 --- a/.pre-commit-config.yaml +++ b/.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] diff --git a/package.json b/package.json index 7d3e448..6a68044 100644 --- a/package.json +++ b/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": [ diff --git a/src/lib/structure/bonding.ts b/src/lib/structure/bonding.ts index 4c1f5da..0c63f93 100644 --- a/src/lib/structure/bonding.ts +++ b/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 = 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 = 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]) + } } } } diff --git a/src/routes/(demos)/molecule/+page.md b/src/routes/(demos)/molecule/+page.md index 17b3c4d..543a287 100644 --- a/src/routes/(demos)/molecule/+page.md +++ b/src/routes/(demos)/molecule/+page.md @@ -30,7 +30,7 @@

{name}

- +