Skip to content

Commit

Permalink
tweak traditional builder
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielYang59 committed Apr 3, 2024
1 parent 48ba33e commit 4ab1721
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
6 changes: 2 additions & 4 deletions cat_scaling/relation/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ def _builder(
return coefs, intercept, metrics

def build_traditional(self, descriptors: Descriptors) -> EadsRelation:
"""Build scaling relations the traditional way, where each adsorbate is
approximated by a single descriptor within each group.
"""Build scaling relations the traditional way, where each adsorbate
is approximated by a single descriptor within each group.
Returns:
Relation: A Relation object containing coefficients and metrics
Expand All @@ -181,8 +181,6 @@ def build_traditional(self, descriptors: Descriptors) -> EadsRelation:

# Get "groups" property from data
groups = descriptors.groups
if groups is None:
raise ValueError("Must set groups before build traditional.")

coefficients_dict = {}
intercepts_dict = {}
Expand Down
24 changes: 16 additions & 8 deletions tests/relation/test_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,9 @@ def setup_data_load(self):
test_df = pd.read_csv(self.test_data_csv, index_col=[0], header=[0])
self.eads = Eads(test_df)

def test_properties(self):
# TODO: need update
pass

def test_invalid_properties(self):
# TODO
pass
def test_invalid_data(self):
with pytest.raises(TypeError, match="Expect data as 'Eads' type"):
Builder(data="data")

def test_build_composite_descriptor(self):
builder = Builder(self.eads)
Expand All @@ -49,6 +45,11 @@ def test_build_composite_descriptor(self):

assert np.allclose(comp_des_2, np.array([0, 0.1, 0.2, 0.3, 0.4, 0.5]))

# Test invalid ratio sum (should sum to 1.0)
with pytest.raises(ValueError, match="Ratios should sum to 1.0"):
builder._build_composite_descriptor({"*A": 0.5, "*D": 0})


def test_builder(self):
# Prepare Builder
builder = Builder(self.eads)
Expand Down Expand Up @@ -76,7 +77,7 @@ def test_build_traditional(self):
builder = Builder(self.eads)

# Define descriptors
descriptors = Descriptors({"*A": ["*B"]})
descriptors = Descriptors(groups={"*A": ["*B"]})

# Test build *B with descriptor *A
relation = builder.build_traditional(descriptors)
Expand All @@ -90,6 +91,13 @@ def test_build_traditional(self):
# Check descriptor ratio
assert isclose(relation.ratios["*B"]["*A"], 1.0, abs_tol=0.01)

def test_build_traditional_invalid(self):
with pytest.raises(ValueError, match="Group member for traditional builder cannot be None"):
descriptors = Descriptors(groups={"*A": None})

builder = Builder(self.eads)
builder.build_traditional(descriptors)

def test_build_adaptive(self):
"""Test build with adaptive descriptor method.
Expand Down

0 comments on commit 4ab1721

Please sign in to comment.