Skip to content

Commit

Permalink
♻️ Refactor/821 refactor model spec (#836)
Browse files Browse the repository at this point in the history
* 🗑️ Deprecated {relations,constraints} -> {clp_relations,clp_constraints)
👌 Improved deprecation messages to users

* ♻️ Change relations -> clp_relations
🩹 Fixes glotaran/builtin/io/yml/test/test_model_parser.py::test_relations

* ♻️ Change constraints-> clp_constraints
🩹 Fixes glotaran/builtin/io/yml/test/test_model_parser.py::test_constraints
  • Loading branch information
jsnel authored Sep 26, 2021
1 parent ace90c5 commit d4677d2
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 44 deletions.
2 changes: 1 addition & 1 deletion glotaran/analysis/test/test_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
def test_constraint(index_dependent, grouped):
model = deepcopy(suite.model)
model.megacomplex["m1"].is_index_dependent = index_dependent
model.constraints.append(ZeroConstraint.from_dict({"target": "s2"}))
model.clp_constraints.append(ZeroConstraint.from_dict({"target": "s2"}))

print("grouped", grouped, "index_dependent", index_dependent)
dataset = simulate(
Expand Down
4 changes: 3 additions & 1 deletion glotaran/analysis/test/test_relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
def test_relations(index_dependent, grouped):
model = deepcopy(suite.model)
model.megacomplex["m1"].is_index_dependent = index_dependent
model.relations.append(Relation.from_dict({"source": "s1", "target": "s2", "parameter": "3"}))
model.clp_relations.append(
Relation.from_dict({"source": "s1", "target": "s2", "parameter": "3"})
)
parameters = ParameterGroup.from_list([11e-4, 22e-5, 2])

print("grouped", grouped, "index_dependent", index_dependent)
Expand Down
12 changes: 6 additions & 6 deletions glotaran/analysis/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,12 @@ def apply_constraints(
index: Any | None,
) -> CalculatedMatrix:

if len(model.constraints) == 0:
if len(model.clp_constraints) == 0:
return matrix

clp_labels = matrix.clp_labels
removed_clp_labels = [
c.target for c in model.constraints if c.target in clp_labels and c.applies(index)
c.target for c in model.clp_constraints if c.target in clp_labels and c.applies(index)
]
reduced_clp_labels = [c for c in clp_labels if c not in removed_clp_labels]
mask = [label in reduced_clp_labels for label in clp_labels]
Expand All @@ -133,14 +133,14 @@ def apply_relations(
index: Any | None,
) -> CalculatedMatrix:

if len(model.relations) == 0:
if len(model.clp_relations) == 0:
return matrix

clp_labels = matrix.clp_labels
relation_matrix = np.diagflat([1.0 for _ in clp_labels])

idx_to_delete = []
for relation in model.relations:
for relation in model.clp_relations:
if relation.target in clp_labels and relation.applies(index):

if relation.source not in clp_labels:
Expand All @@ -166,7 +166,7 @@ def retrieve_clps(
reduced_clps: xr.DataArray,
index: Any | None,
) -> xr.DataArray:
if len(model.relations) == 0 and len(model.constraints) == 0:
if len(model.clp_relations) == 0 and len(model.clp_constraints) == 0:
return reduced_clps

clps = np.zeros(len(clp_labels))
Expand All @@ -175,7 +175,7 @@ def retrieve_clps(
idx = clp_labels.index(label)
clps[idx] = reduced_clps[i]

for relation in model.relations:
for relation in model.clp_relations:
relation = relation.fill(model, parameters)
if (
relation.target in clp_labels
Expand Down
14 changes: 7 additions & 7 deletions glotaran/builtin/io/yml/test/test_model_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ def test_dataset(model):


def test_constraints(model):
print(model.constraints)
assert len(model.constraints) == 2
print(model.clp_constraints)
assert len(model.clp_constraints) == 2

zero = model.constraints[0]
zero = model.clp_constraints[0]
assert isinstance(zero, ZeroConstraint)
assert zero.target == "s1"
assert zero.interval == [[1, 100], [2, 200]]

only = model.constraints[1]
only = model.clp_constraints[1]
assert isinstance(only, OnlyConstraint)
assert only.target == "s1"
assert only.interval == [[1, 100], [2, 200]]
Expand All @@ -88,10 +88,10 @@ def test_penalties(model):


def test_relations(model):
print(model.relations)
assert len(model.relations) == 1
print(model.clp_relations)
assert len(model.clp_relations) == 1

rel = model.relations[0]
rel = model.clp_relations[0]

assert rel.source == "s1"
assert rel.target == "s2"
Expand Down
4 changes: 2 additions & 2 deletions glotaran/builtin/io/yml/test/test_model_spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ megacomplex:
s1: shape1
s2: shape2

constraints:
clp_constraints:
- type: zero
target: s1
interval:
Expand All @@ -92,7 +92,7 @@ clp_area_penalties:
parameter: 55
weight: 0.0016

relations:
clp_relations:
- source: s1
target: s2
parameter: 8
Expand Down
42 changes: 30 additions & 12 deletions glotaran/deprecation/modules/builtin_io_yml.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,27 @@ def model_spec_deprecations(spec: MutableMapping[Any, Any]) -> None:
deprecate_dict_entry(
dict_to_check=spec,
deprecated_usage="spectral_relations",
new_usage="relations",
new_usage="clp_relations",
to_be_removed_in_version="0.7.0",
swap_keys=("spectral_relations", "relations"),
swap_keys=("spectral_relations", "clp_relations"),
stacklevel=load_model_stack_level,
)

if "relations" in spec:
for relation in spec["relations"]:
deprecate_dict_entry(
dict_to_check=spec,
deprecated_usage="relations",
new_usage="clp_relations",
to_be_removed_in_version="0.7.0",
swap_keys=("relations", "clp_relations"),
stacklevel=load_model_stack_level,
)

if "clp_relations" in spec:
for relation in spec["clp_relations"]:
deprecate_dict_entry(
dict_to_check=relation,
deprecated_usage="compartment",
new_usage="source",
deprecated_usage="clp_relations:\n - compartment",
new_usage="clp_relations:\n - source",
to_be_removed_in_version="0.7.0",
swap_keys=("compartment", "source"),
stacklevel=load_model_stack_level,
Expand All @@ -60,18 +69,27 @@ def model_spec_deprecations(spec: MutableMapping[Any, Any]) -> None:
deprecate_dict_entry(
dict_to_check=spec,
deprecated_usage="spectral_constraints",
new_usage="constraints",
new_usage="clp_constraints",
to_be_removed_in_version="0.7.0",
swap_keys=("spectral_constraints", "clp_constraints"),
stacklevel=load_model_stack_level,
)

deprecate_dict_entry(
dict_to_check=spec,
deprecated_usage="constraints",
new_usage="clp_constraints",
to_be_removed_in_version="0.7.0",
swap_keys=("spectral_constraints", "constraints"),
swap_keys=("constraints", "clp_constraints"),
stacklevel=load_model_stack_level,
)

if "constraints" in spec:
for constraint in spec["constraints"]:
if "clp_constraints" in spec:
for constraint in spec["clp_constraints"]:
deprecate_dict_entry(
dict_to_check=constraint,
deprecated_usage="constraint.compartment",
new_usage="constraint.target",
deprecated_usage="clp_constraints:\n - compartment",
new_usage="clp_constraints:\n - target",
to_be_removed_in_version="0.7.0",
swap_keys=("compartment", "target"),
stacklevel=load_model_stack_level,
Expand Down
30 changes: 28 additions & 2 deletions glotaran/deprecation/modules/test/test_builtin_io_yml.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,19 @@
"""
),
3,
"relations",
"clp_relations",
[{"source": "s1"}, {"source": "s2"}],
),
(
dedent(
"""
relations:
- compartment: s1
- compartment: s2
"""
),
3,
"clp_relations",
[{"source": "s1"}, {"source": "s2"}],
),
(
Expand All @@ -41,7 +53,19 @@
"""
),
3,
"constraints",
"clp_constraints",
[{"target": "s1"}, {"target": "s2"}],
),
(
dedent(
"""
constraints:
- compartment: s1
- compartment: s2
"""
),
3,
"clp_constraints",
[{"target": "s1"}, {"target": "s2"}],
),
(
Expand Down Expand Up @@ -86,7 +110,9 @@
"type: kinetic-spectrum",
"type: spectrum",
"spectral_relations",
"relations",
"spectral_constraints",
"constraints",
"equal_area_penalties",
"center_dispersion",
"width_dispersion",
Expand Down
8 changes: 4 additions & 4 deletions glotaran/model/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@

default_model_items = {
"clp_area_penalties": EqualAreaPenalty,
"constraints": Constraint,
"relations": Relation,
"clp_constraints": Constraint,
"clp_relations": Relation,
"weights": Weight,
}

Expand Down Expand Up @@ -254,8 +254,8 @@ def global_megacomplex(self) -> dict[str, Megacomplex]:
return self.megacomplex

def need_index_dependent(self) -> bool:
"""Returns true if e.g. relations with intervals are present."""
return any(i.interval is not None for i in self.constraints + self.relations)
"""Returns true if e.g. clp_relations with intervals are present."""
return any(i.interval is not None for i in self.clp_constraints + self.clp_relations)

def is_groupable(self, parameters: ParameterGroup, data: dict[str, xr.DataArray]) -> bool:
if any(d.has_global_model() for d in self.dataset.values()):
Expand Down
18 changes: 9 additions & 9 deletions glotaran/model/test/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,15 +227,15 @@ def test_model_init():
assert "clp_area_penalties" in model._model_items
assert issubclass(model._model_items["clp_area_penalties"], EqualAreaPenalty)

assert hasattr(model, "constraints")
assert isinstance(model.constraints, list)
assert "constraints" in model._model_items
assert issubclass(model._model_items["constraints"], Constraint)

assert hasattr(model, "relations")
assert isinstance(model.relations, list)
assert "relations" in model._model_items
assert issubclass(model._model_items["relations"], Relation)
assert hasattr(model, "clp_constraints")
assert isinstance(model.clp_constraints, list)
assert "clp_constraints" in model._model_items
assert issubclass(model._model_items["clp_constraints"], Constraint)

assert hasattr(model, "clp_relations")
assert isinstance(model.clp_relations, list)
assert "clp_relations" in model._model_items
assert issubclass(model._model_items["clp_relations"], Relation)

assert hasattr(model, "weights")
assert isinstance(model.weights, list)
Expand Down

0 comments on commit d4677d2

Please sign in to comment.