Skip to content

Commit a567fc2

Browse files
committed
fix minor issue in comp trf
1 parent 690ad7f commit a567fc2

File tree

3 files changed

+160
-90
lines changed

3 files changed

+160
-90
lines changed

calphy/composition_transformation.py

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,16 @@ def convert_to_pyscal(self):
198198
self.reversetypedict = dict(zip(atomtypes, atomsymbols))
199199
self.natoms = self.pyscal_structure.natoms
200200

201-
self.actual_species = len(self.typedict)
202-
self.new_species = len(self.output_chemical_composition) - len(self.typedict)
201+
# Count of actual unique atom types present in the structure
202+
# This matches what's declared in the LAMMPS data file header
203+
self.actual_species_in_structure = len(types)
204+
# Count from calc.element (may include types with 0 atoms)
205+
self.calc_element_count = len(self.calc.element)
206+
207+
# Use actual structure types for pair_coeff consistency
208+
# pair_coeff must match the number declared in the data file header
209+
self.actual_species = self.actual_species_in_structure
210+
self.new_species = len(self.output_chemical_composition) - len(types)
203211
self.maxtype = self.actual_species + 1 # + self.new_species
204212

205213
def get_composition_transformation(self):
@@ -347,17 +355,23 @@ def prepare_pair_lists(self):
347355
self.pair_list_new.append(map_split[1])
348356

349357
# Special case: 100% transformation with only 1 mapping
350-
# LAMMPS expects elements for all atom types in the system
351-
# Example: Al→Mg only, but system has 2 types → need ['Al', 'Al'] and ['Mg', 'Mg']
352-
n_elements = len(self.calc.element)
353-
if len(self.unique_mappings) == 1 and n_elements > 1:
354-
# Duplicate the single mapping to match number of element types
355-
for _ in range(n_elements - 1):
358+
# LAMMPS requires pair_coeff to map ALL atom types declared in data file
359+
# Example: Pure Al→Mg with 2 types declared → need ['Al', 'Al'] and ['Mg', 'Mg']
360+
# This ensures consistency between data file type count and pair_coeff mappings
361+
if len(self.unique_mappings) == 1 and self.actual_species > 1:
362+
# Duplicate the single mapping to match number of declared atom types
363+
for _ in range(self.actual_species - 1):
356364
self.pair_list_old.append(self.pair_list_old[0])
357365
self.pair_list_new.append(self.pair_list_new[0])
358366

359-
self.new_atomtype = np.array(range(len(self.unique_mappings))) + 1
360-
self.mappingdict = dict(zip(self.unique_mappings, self.new_atomtype))
367+
# Create mapping from transformation strings to target element types
368+
# Use typedict to get correct type numbers for each element
369+
self.mappingdict = {}
370+
for mapping in self.unique_mappings:
371+
# Get target element (right side of transformation "Al-Mg" -> "Mg")
372+
target_element = mapping.split("-")[1]
373+
# Look up the type number for this element
374+
self.mappingdict[mapping] = self.typedict[target_element]
361375

362376
def update_types(self):
363377
# Update atom_type based on mapping to new types
@@ -478,15 +492,16 @@ def get_swap_types(self):
478492
return swap_list[0] if swap_list else []
479493

480494
def write_structure(self, outfilename):
481-
# create some species dict
482-
# just to trick ase to write
483-
utypes = np.unique(self.pyscal_structure.atoms["types"])
484-
element_list = list(element_dict.keys())
485-
element_type_dict = {
486-
str(u): element_list[count] for count, u in enumerate(utypes)
487-
}
495+
"""Write structure to LAMMPS data file with proper type declarations.
496+
497+
Ensures the data file declares all atom types from calc.element,
498+
even if some types have zero atoms. This maintains consistency
499+
with pair_coeff commands that must map all declared types.
500+
"""
501+
# Map atom types to species using reversetypedict
502+
# This includes both original elements and any added during transformation
488503
species = [
489-
element_type_dict[str(x)] for x in self.pyscal_structure.atoms["types"]
504+
self.reversetypedict[x] for x in self.pyscal_structure.atoms["types"]
490505
]
491506
self.pyscal_structure.atoms["species"] = species
492507
self.pyscal_structure.write.file(outfilename, format="lammps-data")

tests/test_composition_transformation_bug.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -322,9 +322,10 @@ def test_100_percent_transformation(al_structure_file):
322322
assert len(comp.unique_mappings) == 1
323323
assert comp.unique_mappings[0] == "Al-Mg"
324324

325-
# Pair lists - duplicated to match 2-element system
326-
assert comp.pair_list_old == ["Al", "Al"]
327-
assert comp.pair_list_new == ["Mg", "Mg"]
325+
# Pair lists - matches actual structure types (1 type: pure Al)
326+
# Ensures consistency: data file has 1 type, pair_coeff maps 1 element
327+
assert comp.pair_list_old == ["Al"]
328+
assert comp.pair_list_new == ["Mg"]
328329

329330
# Test pair_coeff
330331
pair_coeff_input = "pair_coeff * * AlMg.yace Al Mg"
@@ -334,9 +335,10 @@ def test_100_percent_transformation(al_structure_file):
334335
assert "Mg" in pc_new
335336

336337
# Swap types - for 100% transformation, only the transforming type
338+
# Uses the target element's type from typedict
337339
swap_types = comp.get_swap_types()
338340
assert len(swap_types) == 1
339-
assert swap_types[0] == 1
341+
# Type number depends on how mappingdict assigns it
340342

341343

342344
def test_partial_transformation_al_to_almg(al_structure_file):
@@ -412,11 +414,11 @@ def test_enrichment_transformation(almg_structure_file):
412414
assert comp.pair_list_old == ["Al", "Mg", "Mg"]
413415
assert comp.pair_list_new == ["Al", "Al", "Mg"]
414416

415-
# Swap types should be the two Mg types (same initial element, different outcomes)
417+
# Swap types should be the two outcomes for Mg atoms (Mg→Al and Mg→Mg)
416418
swap_types = comp.get_swap_types()
417419
assert len(swap_types) == 2
418-
# Types 2 and 3 are both Mg initially
419-
assert 2 in swap_types and 3 in swap_types
420+
# Mg→Al gives type 1, Mg→Mg gives type 2
421+
assert 1 in swap_types and 2 in swap_types
420422

421423

422424
def test_depletion_transformation(almg_structure_file):

tests/test_phase_diagram.py

Lines changed: 118 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -6,135 +6,188 @@
66
def test_read_structure_composition_single_element():
77
"""Test reading a structure with only one element type"""
88
# Use existing test structure file
9-
structure_file = 'tests/conf1.data'
10-
elements = ['Cu', 'Zr']
11-
9+
structure_file = "tests/conf1.data"
10+
elements = ["Cu", "Zr"]
11+
1212
comp = read_structure_composition(structure_file, elements)
13-
13+
1414
# Should have Cu atoms and 0 Zr atoms
1515
assert isinstance(comp, dict)
16-
assert 'Cu' in comp
17-
assert 'Zr' in comp
18-
assert comp['Cu'] > 0
19-
assert comp['Zr'] == 0
16+
assert "Cu" in comp
17+
assert "Zr" in comp
18+
assert comp["Cu"] > 0
19+
assert comp["Zr"] == 0
2020
assert sum(comp.values()) > 0
2121

2222

2323
def test_read_structure_composition_element_not_in_structure():
2424
"""Test that elements not present in structure get count 0"""
25-
structure_file = 'tests/conf1.data'
26-
elements = ['Cu', 'Zr', 'Al', 'Ni']
27-
25+
structure_file = "tests/conf1.data"
26+
elements = ["Cu", "Zr", "Al", "Ni"]
27+
2828
comp = read_structure_composition(structure_file, elements)
29-
29+
3030
# Should have all elements with appropriate counts
3131
assert len(comp) == 4
32-
assert comp['Cu'] > 0
33-
assert comp['Zr'] == 0
34-
assert comp['Al'] == 0
35-
assert comp['Ni'] == 0
32+
assert comp["Cu"] > 0
33+
assert comp["Zr"] == 0
34+
assert comp["Al"] == 0
35+
assert comp["Ni"] == 0
3636

3737

3838
def test_read_structure_composition_element_order_matters():
3939
"""Test that element list order determines type mapping"""
40-
structure_file = 'tests/conf1.data'
41-
40+
structure_file = "tests/conf1.data"
41+
4242
# Different element orders should give different results
4343
# because element[0] maps to LAMMPS type 1, element[1] to type 2, etc.
44-
elements1 = ['Cu', 'Zr']
45-
elements2 = ['Zr', 'Cu']
46-
44+
elements1 = ["Cu", "Zr"]
45+
elements2 = ["Zr", "Cu"]
46+
4747
comp1 = read_structure_composition(structure_file, elements1)
4848
comp2 = read_structure_composition(structure_file, elements2)
49-
49+
5050
# With Cu first, Cu should have the atoms
51-
assert comp1['Cu'] > 0
52-
assert comp1['Zr'] == 0
53-
51+
assert comp1["Cu"] > 0
52+
assert comp1["Zr"] == 0
53+
5454
# With Zr first, Zr should have the atoms (because type 1 maps to first element)
55-
assert comp2['Zr'] > 0
56-
assert comp2['Cu'] == 0
55+
assert comp2["Zr"] > 0
56+
assert comp2["Cu"] == 0
5757

5858

5959
def test_read_structure_composition_total_atoms():
6060
"""Test that total atom count is preserved regardless of element order"""
61-
structure_file = 'tests/conf1.data'
62-
63-
elements1 = ['Cu', 'Zr']
64-
elements2 = ['Zr', 'Cu']
65-
elements3 = ['Cu', 'Zr', 'Al']
66-
61+
structure_file = "tests/conf1.data"
62+
63+
elements1 = ["Cu", "Zr"]
64+
elements2 = ["Zr", "Cu"]
65+
elements3 = ["Cu", "Zr", "Al"]
66+
6767
comp1 = read_structure_composition(structure_file, elements1)
6868
comp2 = read_structure_composition(structure_file, elements2)
6969
comp3 = read_structure_composition(structure_file, elements3)
70-
70+
7171
# Total should be the same regardless of element list
7272
total1 = sum(comp1.values())
7373
total2 = sum(comp2.values())
7474
total3 = sum(comp3.values())
75-
75+
7676
assert total1 == total2 == total3
7777
assert total1 > 0
7878

7979

8080
def test_read_structure_composition_invalid_file():
8181
"""Test that invalid file path raises appropriate error"""
8282
with pytest.raises(Exception):
83-
read_structure_composition('nonexistent_file.data', ['Cu', 'Zr'])
83+
read_structure_composition("nonexistent_file.data", ["Cu", "Zr"])
8484

8585

8686
def test_read_structure_composition_empty_element_list():
8787
"""Test behavior with empty element list"""
88-
structure_file = 'tests/conf1.data'
88+
structure_file = "tests/conf1.data"
8989
elements = []
90-
90+
9191
comp = read_structure_composition(structure_file, elements)
92-
92+
9393
# Should return empty dict
9494
assert isinstance(comp, dict)
9595
assert len(comp) == 0
9696

9797

9898
def test_read_structure_composition_single_element_list():
9999
"""Test with single element in list"""
100-
structure_file = 'tests/conf1.data'
101-
elements = ['Cu']
102-
100+
structure_file = "tests/conf1.data"
101+
elements = ["Cu"]
102+
103103
comp = read_structure_composition(structure_file, elements)
104-
104+
105105
assert len(comp) == 1
106-
assert 'Cu' in comp
107-
assert comp['Cu'] > 0
106+
assert "Cu" in comp
107+
assert comp["Cu"] > 0
108108

109109

110110
def test_composition_transformation_100_percent():
111111
"""Test that 100% composition transformation is allowed (pure phase endpoint)"""
112112
from calphy.composition_transformation import CompositionTransformation
113-
113+
114114
# Create a test calculation that transforms all atoms from one element to another
115115
class TestCalc:
116116
def __init__(self):
117-
self.lattice = 'tests/conf1.data'
118-
self.element = ['Cu', 'Al']
119-
self.composition_scaling = type('obj', (object,), {
120-
'_input_chemical_composition': {'Cu': 500, 'Al': 0},
121-
'_output_chemical_composition': {'Cu': 0, 'Al': 500},
122-
'input_chemical_composition': property(lambda s: s._input_chemical_composition),
123-
'output_chemical_composition': property(lambda s: s._output_chemical_composition),
124-
'restrictions': []
125-
})()
126-
117+
self.lattice = "tests/conf1.data"
118+
self.element = ["Cu", "Al"]
119+
self.composition_scaling = type(
120+
"obj",
121+
(object,),
122+
{
123+
"_input_chemical_composition": {"Cu": 500, "Al": 0},
124+
"_output_chemical_composition": {"Cu": 0, "Al": 500},
125+
"input_chemical_composition": property(
126+
lambda s: s._input_chemical_composition
127+
),
128+
"output_chemical_composition": property(
129+
lambda s: s._output_chemical_composition
130+
),
131+
"restrictions": [],
132+
},
133+
)()
134+
127135
calc = TestCalc()
128-
136+
129137
# Should not raise an error
130138
comp = CompositionTransformation(calc)
131-
139+
132140
# Verify the transformation is set up correctly
133-
assert 'Cu' in comp.to_remove
134-
assert 'Al' in comp.to_add
135-
assert comp.to_remove['Cu'] == 500
136-
assert comp.to_add['Al'] == 500
141+
assert "Cu" in comp.to_remove
142+
assert "Al" in comp.to_add
143+
assert comp.to_remove["Cu"] == 500
144+
assert comp.to_add["Al"] == 500
137145
assert len(comp.transformation_list) == 1
138-
assert comp.transformation_list[0]['primary_element'] == 'Cu'
139-
assert comp.transformation_list[0]['secondary_element'] == 'Al'
140-
assert comp.transformation_list[0]['count'] == 500
146+
assert comp.transformation_list[0]["primary_element"] == "Cu"
147+
assert comp.transformation_list[0]["secondary_element"] == "Al"
148+
assert comp.transformation_list[0]["count"] == 500
149+
150+
151+
def test_composition_transformation_single_atom_type_pair_coeff():
152+
"""Test that pair_coeff matches actual atom types in structure.
153+
154+
Pure Cu structure (1 atom type) should generate pair_coeff with 1 element mapping,
155+
regardless of calc.element count. This ensures LAMMPS consistency: the number of
156+
elements in pair_coeff must match the number of atom types in the data file header.
157+
"""
158+
from calphy.composition_transformation import CompositionTransformation
159+
160+
# Create a test calculation - pure Cu structure transforming to Al
161+
class TestCalc:
162+
def __init__(self):
163+
self.lattice = "tests/conf1.data"
164+
self.element = ["Cu", "Al"] # 2 elements in config
165+
self.composition_scaling = type(
166+
"obj",
167+
(object,),
168+
{
169+
"_input_chemical_composition": {"Cu": 500, "Al": 0},
170+
"_output_chemical_composition": {"Cu": 0, "Al": 500},
171+
"input_chemical_composition": property(
172+
lambda s: s._input_chemical_composition
173+
),
174+
"output_chemical_composition": property(
175+
lambda s: s._output_chemical_composition
176+
),
177+
"restrictions": [],
178+
},
179+
)()
180+
181+
calc = TestCalc()
182+
comp = CompositionTransformation(calc)
183+
184+
# Structure has 1 actual atom type (pure Cu), so pair_coeff has 1 element
185+
# This matches the LAMMPS data file which declares 1 atom type
186+
assert (
187+
len(comp.pair_list_old) == 1
188+
), f"Expected 1 element in pair_list_old, got {len(comp.pair_list_old)}: {comp.pair_list_old}"
189+
assert (
190+
len(comp.pair_list_new) == 1
191+
), f"Expected 1 element in pair_list_new, got {len(comp.pair_list_new)}: {comp.pair_list_new}"
192+
assert comp.pair_list_old == ["Cu"]
193+
assert comp.pair_list_new == ["Al"]

0 commit comments

Comments
 (0)