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

add spin for abacus/stru #751

Merged
merged 5 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion dpdata/abacus/md.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def get_frame(fname):
with open_file(geometry_path_in) as fp:
geometry_inlines = fp.read().split("\n")
celldm, cell = get_cell(geometry_inlines)
atom_names, natoms, types, coords, move = get_coords(
atom_names, natoms, types, coords, move, magmom = get_coords(
celldm, cell, geometry_inlines, inlines
)
# This coords is not to be used.
Expand Down
2 changes: 1 addition & 1 deletion dpdata/abacus/relax.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def get_frame(fname):
with open_file(geometry_path_in) as fp:
geometry_inlines = fp.read().split("\n")
celldm, cell = get_cell(geometry_inlines)
atom_names, natoms, types, coord_tmp, move = get_coords(
atom_names, natoms, types, coord_tmp, move, magmom = get_coords(
celldm, cell, geometry_inlines, inlines
)

Expand Down
71 changes: 61 additions & 10 deletions dpdata/abacus/scf.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,51 @@ def parse_stru_pos(pos_line):
return pos, move, velocity, magmom, angle1, angle2, constrain, lambda1


def get_atom_mag_cartesian(atommag, angle1, angle2):
"""Transform atommag, angle1, angle2 to magmom in cartesian coordinates.

Parameters
----------
atommag : float/list of float/None
Atom magnetic moment.
angle1 : float/None
value of angle1.
angle2 : float/None
value of angle2.
ABACUS support defining mag, angle1, angle2 at the same time.
angle1 is the angle between z-axis and real spin (in degrees).
angle2 is the angle between x-axis and real spin projection in xy-plane (in degrees).
If only mag is defined, then transfer it to magmom directly.
And if mag, angle1, angle2 are defined, then mag is only the norm of magmom, and the direction is defined by angle1 and angle2.
"""
if atommag is None:
return None
if not (isinstance(atommag, list) or isinstance(atommag, float)):
raise RuntimeError(f"Invalid atommag: {atommag}")

if angle1 is None and angle2 is None:
if isinstance(atommag, list):
return atommag
else:
return [0, 0, atommag]
else:
a1 = 0
a2 = 0
if angle1 is not None:
a1 = angle1
if angle2 is not None:
a2 = angle2
if isinstance(atommag, list):
mag_norm = np.linalg.norm(atommag)
else:
mag_norm = atommag
return [
mag_norm * np.sin(np.radians(a1)) * np.cos(np.radians(a2)),
mag_norm * np.sin(np.radians(a1)) * np.sin(np.radians(a2)),
mag_norm * np.cos(np.radians(a1)),
]


def get_coords(celldm, cell, geometry_inlines, inlines=None):
coords_lines = get_stru_block(geometry_inlines, "ATOMIC_POSITIONS")
# assuming that ATOMIC_POSITIONS is at the bottom of the STRU file
Expand All @@ -268,16 +313,15 @@ def get_coords(celldm, cell, geometry_inlines, inlines=None):
coords = [] # coordinations of atoms
move = [] # move flag of each atom
velocity = [] # velocity of each atom
mag = [] # magnetic moment of each atom
angle1 = [] # angle1 of each atom
angle2 = [] # angle2 of each atom
mags = [] # magnetic moment of each atom
sc = [] # spin constraint flag of each atom
lambda_ = [] # lambda of each atom

ntype = get_nele_from_stru(geometry_inlines)
line_idx = 1 # starting line of first element
for it in range(ntype):
atom_names.append(coords_lines[line_idx].split()[0])
atom_type_mag = float(coords_lines[line_idx + 1].split()[0])
line_idx += 2
atom_numbs.append(int(coords_lines[line_idx].split()[0]))
line_idx += 1
Expand All @@ -302,17 +346,20 @@ def get_coords(celldm, cell, geometry_inlines, inlines=None):
if imove is not None:
move.append(imove)
velocity.append(ivelocity)
mag.append(imagmom)
angle1.append(iangle1)
angle2.append(iangle2)
sc.append(iconstrain)
lambda_.append(ilambda1)

# calculate the magnetic moment in cartesian coordinates
mag = get_atom_mag_cartesian(imagmom, iangle1, iangle2)
if mag is None:
mag = [0, 0, atom_type_mag]
mags.append(mag)

line_idx += 1
coords = np.array(coords) # need transformation!!!
atom_types = np.array(atom_types)
move = np.array(move, dtype=bool)
return atom_names, atom_numbs, atom_types, coords, move
return atom_names, atom_numbs, atom_types, coords, move, mags


def get_energy(outlines):
Expand Down Expand Up @@ -479,9 +526,12 @@ def get_frame(fname):
outlines = fp.read().split("\n")

celldm, cell = get_cell(geometry_inlines)
atom_names, natoms, types, coords, move = get_coords(
celldm, cell, geometry_inlines, inlines
atom_names, natoms, types, coords, move, magmom = (
get_coords( # here the magmom is the initial magnetic moment in STRU
celldm, cell, geometry_inlines, inlines
)
)

magmom, magforce = get_mag_force(outlines)
if len(magmom) > 0:
magmom = magmom[-1:]
Expand Down Expand Up @@ -565,7 +615,7 @@ def get_frame_from_stru(fname):
nele = get_nele_from_stru(geometry_inlines)
inlines = [f"ntype {nele}"]
celldm, cell = get_cell(geometry_inlines)
atom_names, natoms, types, coords, move = get_coords(
atom_names, natoms, types, coords, move, magmom = get_coords(
celldm, cell, geometry_inlines, inlines
)
data = {}
Expand All @@ -575,6 +625,7 @@ def get_frame_from_stru(fname):
data["cells"] = cell[np.newaxis, :, :]
data["coords"] = coords[np.newaxis, :, :]
data["orig"] = np.zeros(3)
data["spins"] = np.array([magmom])
if len(move) > 0:
data["move"] = move[np.newaxis, :, :]

Expand Down
4 changes: 4 additions & 0 deletions dpdata/plugins/abacus.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
@Format.register("stru")
class AbacusSTRUFormat(Format):
def from_system(self, file_name, **kwargs):
data = dpdata.abacus.scf.get_frame(file_name)
register_mag_data(data)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Review data handling flow in from_system

The current implementation has potential issues:

  1. get_frame is called to get data for magnetic registration
  2. Then get_frame_from_stru is called to return the actual frame
  3. This double data fetch is inefficient and could lead to inconsistencies

Consider refactoring to:

 def from_system(self, file_name, **kwargs):
-    data = dpdata.abacus.scf.get_frame(file_name)
-    register_mag_data(data)
-    return dpdata.abacus.scf.get_frame_from_stru(file_name)
+    data = dpdata.abacus.scf.get_frame_from_stru(file_name)
+    register_mag_data(data)
+    return data
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
data = dpdata.abacus.scf.get_frame(file_name)
register_mag_data(data)
data = dpdata.abacus.scf.get_frame_from_stru(file_name)
register_mag_data(data)
return data

return dpdata.abacus.scf.get_frame_from_stru(file_name)

def to_system(self, data, file_name: FileType, frame_idx=0, **kwargs):
Expand Down Expand Up @@ -55,6 +57,7 @@ def register_mag_data(data):
required=False,
deepmd_name="spin",
)
dpdata.System.register_data_type(dt)
dpdata.LabeledSystem.register_data_type(dt)
if "mag_forces" in data:
dt = DataType(
Expand All @@ -64,6 +67,7 @@ def register_mag_data(data):
required=False,
deepmd_name="force_mag",
)
dpdata.System.register_data_type(dt)
dpdata.LabeledSystem.register_data_type(dt)


Expand Down
24 changes: 24 additions & 0 deletions tests/abacus.spin/STRU.spin
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
ATOMIC_SPECIES
Fe 55.845 Fe.upf

NUMERICAL_ORBITAL
Fe_gga_10au_200.0Ry_4s2p2d1f.orb

LATTICE_CONSTANT
1.880277359
LATTICE_VECTORS
2.8274254848 0.0000000000 0.0000000000 #latvec1
0.0000000000 2.8274254848 0.0000000000 #latvec2
0.0000000000 0.0000000000 2.8274254848 #latvec3

ATOMIC_POSITIONS
Direct

Fe #label
1 #magnetism
4 #number of atoms
0.0000000000 0.000000000 0.000000000 mag 0 0 2
0.1000000000 0.1000000000 0.1000000000 mag 3
0.2000000000 0.2000000000 0.2000000000 mag 3 angle1 90
0.3000000000 0.3000000000 0.3000000000 mag 3 4 0 angle1 90 angle2 90

16 changes: 16 additions & 0 deletions tests/test_abacus_spin.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,19 @@ def test_md(self):
np.testing.assert_almost_equal(
data["mag_forces"], sys2.data["mag_forces"], decimal=8
)

def test_read_stru_spin(self):
mysys = dpdata.System("abacus.spin/STRU.spin", fmt="abacus/stru")
self.assertTrue("spins" in mysys.data)
Comment on lines +160 to +161
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Path should be relative to test directory

The test file exists at tests/abacus.spin/STRU.spin, but the test is using abacus.spin/STRU.spin. This could cause issues when running tests from different directories. The test file path should be relative to the test directory.

  • Update the path in tests/test_abacus_spin.py:160 to use "./abacus.spin/STRU.spin" or use proper test fixtures/helpers to handle test file paths.
🔗 Analysis chain

Verify test file existence and handling.

Let's verify the test file setup and error handling.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify test file existence and contents
if [ -f "abacus.spin/STRU.spin" ]; then
    echo "Test file exists"
    head -n 10 "abacus.spin/STRU.spin"
else
    echo "Warning: Test file not found"
fi

# Check for similar test files in the repository
fd -e spin "STRU"

Length of output: 176


Script:

#!/bin/bash
# Check the test file content in the correct test directory
cat tests/abacus.spin/STRU.spin

# Check if the test is using relative path correctly
rg -A 5 "abacus.spin/STRU.spin" tests/test_abacus_spin.py

# Check if there are other similar test file usages
rg "STRU.spin" tests/

Length of output: 1084

print(mysys.data["spins"])

"""
0.0000000000 0.000000000 0.000000000 mag 0 0 2
0.1000000000 0.1000000000 0.1000000000 mag 3
0.2000000000 0.2000000000 0.2000000000 mag 3 angle1 90
0.3000000000 0.3000000000 0.3000000000 mag 3 4 0 angle1 90 angle2 90
"""
np.testing.assert_almost_equal(mysys.data["spins"][0][0], [0, 0, 2], decimal=8)
np.testing.assert_almost_equal(mysys.data["spins"][0][1], [0, 0, 3], decimal=8)
np.testing.assert_almost_equal(mysys.data["spins"][0][2], [3, 0, 0], decimal=8)
np.testing.assert_almost_equal(mysys.data["spins"][0][3], [0, 5, 0], decimal=8)
Loading