Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: xyz file parsing - nAtoms & nLines to check formatting. Created… #52

Open
wants to merge 13 commits into
base: epic/SOF-5386
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/cicd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ jobs:
steps:
Copy link
Contributor Author

@adewyer adewyer Jan 13, 2022

Choose a reason for hiding this comment

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

Fixes from @tjduigna to fix CICD test failures caused by git lfs not pulling test files.

- name: Checkout this repository
uses: actions/checkout@v2
with:
lfs: true

- name: Checkout actions repository
uses: actions/checkout@v2
Expand All @@ -40,6 +42,8 @@ jobs:
steps:
- name: Checkout this repository
uses: actions/checkout@v2
with:
lfs: true

- name: Checkout actions repository
uses: actions/checkout@v2
Expand Down
56 changes: 56 additions & 0 deletions src/default_material.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { ATOMIC_COORD_UNITS, units } from "./constants";
import { LATTICE_TYPE } from "./lattice/types";

export const defaultMaterialConfig = {
name: "Silicon FCC",
basis: {
elements: [
{
id: 1,
value: "Si",
},
{
id: 2,
value: "Si",
},
],
coordinates: [
{
id: 1,
value: [0.0, 0.0, 0.0],
},
{
id: 2,
value: [0.25, 0.25, 0.25],
},
],
units: ATOMIC_COORD_UNITS.crystal,
},
lattice: {
// Primitive cell for Diamond FCC Silicon at ambient conditions
type: LATTICE_TYPE.FCC,
Copy link
Member

Choose a reason for hiding this comment

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

I suspect the fact that we are using LATTICE_TYPE here and in the Lattice, and then import Lattice inside Material could lead to circular dependency which could lead to the problem with tests for https://github.com/Exabyte-io/exabyte-stack/pull/235/files#r805085142

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay i will look into this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We saw these same errors prior to me moving default materials out of material.

Previously we imported both LATTICE_TYPE & LATTICE into material.js. Wouldn't that lead to the same circular error?

a: 3.867,
b: 3.867,
c: 3.867,
alpha: 60,
beta: 60,
gamma: 60,
units: {
length: units.angstrom,
angle: units.degree,
},
},
};

/*
* Function returns the defaultMaterial object.
* @returns {Object}
*/
export function getDefaultMaterialConfig() {
return defaultMaterialConfig;
Copy link
Member

Choose a reason for hiding this comment

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

return {...defaultMaterialConfig}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: utilize this function instead of defaultMaterialConfig in code.

}

export default {
defaultMaterialConfig,
getDefaultMaterialConfig,
};
4 changes: 3 additions & 1 deletion src/made.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ import { ArrayWithIds } from "./abstract/array_with_ids";
import { Basis } from "./basis/basis";
import { ATOMIC_COORD_UNITS, coefficients, tolerance, units } from "./constants";
import { AtomicConstraints } from "./constraints/constraints";
import { defaultMaterialConfig, getDefaultMaterialConfig } from "./default_material";
import { Lattice, nonPeriodicLatticeScalingFactor } from "./lattice/lattice";
import { ReciprocalLattice } from "./lattice/reciprocal/lattice_reciprocal";
import { DEFAULT_LATTICE_UNITS, LATTICE_TYPE_CONFIGS } from "./lattice/types";
import { defaultMaterialConfig, Material } from "./material";
import { Material } from "./material";
import MadeMath from "./math";
import parsers from "./parsers/parsers";
import tools from "./tools/index";
Expand All @@ -19,6 +20,7 @@ export const Made = {

Material,
defaultMaterialConfig,
getDefaultMaterialConfig,
Lattice,
nonPeriodicLatticeScalingFactor,
ReciprocalLattice,
Expand Down
63 changes: 10 additions & 53 deletions src/material.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,53 +7,10 @@ import {
PRIMITIVE_TO_CONVENTIONAL_CELL_LATTICE_TYPES,
PRIMITIVE_TO_CONVENTIONAL_CELL_MULTIPLIERS,
} from "./cell/conventional_cell";
import { ATOMIC_COORD_UNITS, units } from "./constants";
import { Lattice } from "./lattice/lattice";
import { LATTICE_TYPE } from "./lattice/types";
import parsers from "./parsers/parsers";
import supercellTools from "./tools/supercell";

export const defaultMaterialConfig = {
name: "Silicon FCC",
basis: {
elements: [
{
id: 1,
value: "Si",
},
{
id: 2,
value: "Si",
},
],
coordinates: [
{
id: 1,
value: [0.0, 0.0, 0.0],
},
{
id: 2,
value: [0.25, 0.25, 0.25],
},
],
units: ATOMIC_COORD_UNITS.crystal,
},
lattice: {
// Primitive cell for Diamond FCC Silicon at ambient conditions
type: LATTICE_TYPE.FCC,
a: 3.867,
b: 3.867,
c: 3.867,
alpha: 60,
beta: 60,
gamma: 60,
units: {
length: units.angstrom,
angle: units.degree,
},
},
};

export class Material {
constructor(config) {
this._json = lodash.cloneDeep(config || {});
Expand Down Expand Up @@ -111,15 +68,15 @@ export class Material {
* @returns {Object}
*/
getDerivedPropertyByName(name) {
return this.getDerivedProperties().find(x => x.name === name);
return this.getDerivedProperties().find((x) => x.name === name);
}

/**
* @summary Returns the derived properties array for a material.
* @returns {Array}
*/
getDerivedProperties() {
return this.prop('derivedProperties', []);
return this.prop("derivedProperties", []);
}

/**
Expand Down Expand Up @@ -197,12 +154,11 @@ export class Material {
* @returns {String}
*/
getInchiStringForHash() {
const inchi = this.getDerivedPropertyByName('inchi');
const inchi = this.getDerivedPropertyByName("inchi");
if (inchi) {
return inchi.value
} else {
throw new Error("Hash cannot be created. Missing InChI string in derivedProperties")
return inchi.value;
}
throw new Error("Hash cannot be created. Missing InChI string in derivedProperties");
}

/**
Expand All @@ -215,22 +171,23 @@ export class Material {
* @param salt {String} Salt for hashing, empty string by default.
* @param isScaled {Boolean} Whether to scale the lattice parameter 'a' to 1.
*/
calculateHash(salt = '', isScaled = false) {
calculateHash(salt = "", isScaled = false) {
let message;
if (!this.isNonPeriodic) {
message = this.Basis.hashString + "#" + this.Lattice.getHashString(isScaled) + "#" + salt;
message =
this.Basis.hashString + "#" + this.Lattice.getHashString(isScaled) + "#" + salt;
} else {
message = this.getInchiStringForHash();
}
return CryptoJS.MD5(message).toString();
}

set hash(hash) {
this.setProp('hash', hash);
this.setProp("hash", hash);
}

get hash() {
return this.prop('hash');
return this.prop("hash");
}

/**
Expand Down
18 changes: 18 additions & 0 deletions src/parsers/parsers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,27 @@ import espresso from "./espresso";
import poscar from "./poscar";
import xyz from "./xyz";

/**
* Function returns the number of atoms in a file using the proper parser function based on the file extension.
* @param {String} fileExtension
* @param {String} fileContent
* @returns {Number}
*/
export function getNumberOfAtomsInFileByExtension(fileExtension, fileContent) {
let numberOfAtoms = 0;
if (fileExtension === "poscar") {
numberOfAtoms = poscar.getAtomsCount(fileContent);
}
if (fileExtension === "xyz") {
numberOfAtoms = xyz.getAtomsCount(fileContent);
}
return numberOfAtoms;
}

export default {
xyz,
poscar,
cif,
espresso,
getNumberOfAtomsInFileByExtension,
};
37 changes: 35 additions & 2 deletions src/parsers/poscar.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import s from "underscore.string";

import { ConstrainedBasis } from "../basis/constrained_basis";
import { ATOMIC_COORD_UNITS } from "../constants";
import { getDefaultMaterialConfig } from "../default_material";
import { Lattice } from "../lattice/lattice";
import math from "../math";
import xyz from "./xyz";

const _print = (x, printFormat = "%14.9f") => s.sprintf(printFormat, math.precise(x));
const _latticeVectorsToString = (vectors) =>
Expand Down Expand Up @@ -58,13 +60,44 @@ function toPoscar(materialOrConfig, omitConstraints = false) {
* @param poscarFileContent
* @returns {Number}
*/
export function atomsCount(poscarFileContent) {
export function getAtomsCount(poscarFileContent) {
const atomsLine = poscarFileContent.split("\n")[6].split(" ");
return atomsLine.map((x) => parseInt(x, 10)).reduce((a, b) => a + b);
}

/**
* Converts XYZ formated string content to POSCAR formated string content.
* @param xyzContent
* @returns {string}
*/
export function convertFromXyz(xyzContent) {
const xyzConfig = { ...getDefaultMaterialConfig() };
const xyzArray = xyzContent.split(/\r?\n/);
const xyzArrayBasisOnly = xyzArray.slice(2, -1);
const xyzBasis = xyzArrayBasisOnly.join("\n");
xyzConfig.basis = xyz.toBasisConfig(xyzBasis);
xyzConfig.basis.units = "cartesian";
return toPoscar(xyzConfig);
}

/**
* Function converts a string of content into a POSCAR formatted string of content. The function called to convert the
* format is determined by the format type passed to the function.
* @param {String} xyzContent
* @param {String} format
* @returns {String}
*/
export function convertFromOtherFormat(content, format) {
if (format === "xyz") {
return convertFromXyz(content);
}
return content;
}

export default {
toPoscar,
atomicConstraintsCharFromBool,
atomsCount,
getAtomsCount,
convertFromOtherFormat,
convertFromXyz,
};
11 changes: 11 additions & 0 deletions src/parsers/xyz.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,21 @@ function fromMaterial(materialOrConfig, fractional = false) {
return fromBasis(basis, "%11.6f");
}

/**
* Function splits the xyzFile string at new lines and then returns the first element of the array.
* The first line of the XYZ file should contain the number of atoms in the structure.
* @param {String} xyzFile
* @returns {Number}
*/
export function getAtomsCount(xyzFile) {
return parseInt(xyzFile.split(/\r?\n/)[0], 10);
}

export default {
validate,
fromMaterial,
toBasisConfig,
fromBasis,
CombinatorialBasis,
getAtomsCount,
};
3 changes: 3 additions & 0 deletions tests/enums.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ export const SiPWSCFInput = readFile(path.join(FIXTURES_DIR, "Si-pwscf.in"));
export const Zr1H23Zr1H1 = readJSONFile(path.join(FIXTURES_DIR, "Zr1H23Zr1H1.json"));
export const Zr1H23Zr1H1Poscar = readFile(path.join(FIXTURES_DIR, "Zr1H23Zr1H1.poscar"));
export const H2O = readFile(path.join(FIXTURES_DIR, "H2O.poscar"));

export const CH4 = readFile(path.join(FIXTURES_DIR, "CH4.xyz"));
export const CH4POSCAR = readFile(path.join(FIXTURES_DIR, "CH4.poscar"));
3 changes: 3 additions & 0 deletions tests/fixtures/CH4.poscar
Git LFS file not shown
3 changes: 3 additions & 0 deletions tests/fixtures/CH4.xyz
Git LFS file not shown
18 changes: 15 additions & 3 deletions tests/parsers/poscar.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
import { expect } from "chai";

import { Material } from "../../src/material";
import { atomsCount } from "../../src/parsers/poscar";
import { H2O, Na4Cl4, Na4Cl4Poscar, Zr1H23Zr1H1, Zr1H23Zr1H1Poscar } from "../enums";
import { convertFromOtherFormat, getAtomsCount } from "../../src/parsers/poscar";
import {
CH4,
CH4POSCAR,
H2O,
Na4Cl4,
Na4Cl4Poscar,
Zr1H23Zr1H1,
Zr1H23Zr1H1Poscar,
} from "../enums";
import { assertDeepAlmostEqual } from "../utils";

describe("Parsers.POSCAR", () => {
it("should return a valid poscar", () => {
Expand All @@ -16,6 +25,9 @@ describe("Parsers.POSCAR", () => {
});

it("should return the number of atoms for a molecule in a poscar file", () => {
expect(atomsCount(H2O)).to.be.equal(3);
expect(getAtomsCount(H2O)).to.be.equal(3);
});
it("should return the xyz file content in poscar file format", () => {
assertDeepAlmostEqual(convertFromOtherFormat(CH4, "xyz"), CH4POSCAR);
});
});
8 changes: 7 additions & 1 deletion tests/parsers/xyz.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { expect } from "chai";

import parsers from "../../src/parsers/parsers";
import { Si } from "../enums";
import { getAtomsCount } from "../../src/parsers/xyz";
import { CH4, Si } from "../enums";
import { assertDeepAlmostEqual } from "../utils";

describe("Parsers:XYZ", () => {
Expand All @@ -11,4 +14,7 @@ describe("Parsers:XYZ", () => {
"units",
]);
});
it("should return the number of atoms for a molecule in an xyz file", () => {
expect(getAtomsCount(CH4)).to.be.equal(5);
});
});