Remove criterion exclude attribute, use logical criterion combination with not instead

execution_engine/converter/action/abstract.py

@@ -143,7 +143,7 @@ def _to_criterion(self) -> Criterion | LogicalCriterionCombination | None:
         raise NotImplementedError()
 
     @final
-    def to_criterion(self) -> Criterion | LogicalCriterionCombination:
+    def to_positive_criterion(self) -> Criterion | LogicalCriterionCombination:
         """
         Converts this action to a criterion.
         """
@@ -155,21 +155,14 @@ def to_criterion(self) -> Criterion | LogicalCriterionCombination:
             ), "Action without explicit criterion must have at least one goal"
 
         if self.goals:
-            combination = LogicalCriterionCombination(
-                exclude=self._exclude,  # need to pull up the exclude flag from the criterion into the combination
-                category=CohortCategory.INTERVENTION,
-                operator=LogicalCriterionCombination.Operator("AND"),
-            )
+            criteria = [goal.to_criterion() for goal in self.goals]
             if action is not None:
-                action.exclude = False  # reset the exclude flag, as it is now part of the combination
-                combination.add(action)
-
-            for goal in self.goals:
-                combination.add(goal.to_criterion())
-
-            return combination
-
-        return action  # type: ignore
+                criteria.append(action)
+            return LogicalCriterionCombination.And(
+                *criteria, category=CohortCategory.INTERVENTION
+            )
+        else:
+            return action  # type: ignore
 
     @property
     def goals(self) -> list[Goal]:

execution_engine/converter/action/assessment.py

@@ -19,7 +19,7 @@
 
 class AssessmentAction(AbstractAction):
     """
-    An AsessmentAction is an action that is used to assess a patient's condition.
+    An AssessmentAction is an action that is used to assess a patient's condition.
 
     This action just tests whether the assessment has been performed by determining whether any value
     is present in the respective OMOP CDM table.
@@ -79,7 +79,6 @@ def _to_criterion(self) -> Criterion | LogicalCriterionCombination | None:
                 )
 
         criterion = cls(
-            exclude=self._exclude,
             category=CohortCategory.INTERVENTION,
             concept=self._code,
             timing=self._timing,

execution_engine/converter/action/body_positioning.py

@@ -53,7 +53,6 @@ def _to_criterion(self) -> Criterion | LogicalCriterionCombination | None:
         """Converts this characteristic to a Criterion."""
 
         return ProcedureOccurrence(
-            exclude=self._exclude,
             category=CohortCategory.INTERVENTION,
             concept=self._code,
             timing=self._timing,

execution_engine/converter/action/drug_administration.py

@@ -16,6 +16,7 @@
 from execution_engine.fhir.recommendation import RecommendationPlan
 from execution_engine.omop.concepts import Concept
 from execution_engine.omop.criterion.abstract import Criterion
+from execution_engine.omop.criterion.combination.combination import CriterionCombination
 from execution_engine.omop.criterion.combination.logical import (
     LogicalCriterionCombination,
     NonCommutativeLogicalCriterionCombination,
@@ -284,7 +285,6 @@ def _to_criterion(self) -> Criterion | LogicalCriterionCombination | None:
         if not self._dosages:
             # no dosages, just return the drug exposure
             return DrugExposure(
-                exclude=self._exclude,
                 category=CohortCategory.INTERVENTION,
                 ingredient_concept=self._ingredient_concept,
                 dose=None,
@@ -293,7 +293,6 @@ def _to_criterion(self) -> Criterion | LogicalCriterionCombination | None:
 
         for dosage in self._dosages:
             drug_action = DrugExposure(
-                exclude=False,  # first set to False, as the exclude flag is pulled up into the combination
                 category=CohortCategory.INTERVENTION,
                 ingredient_concept=self._ingredient_concept,
                 dose=dosage["dose"],
@@ -316,33 +315,31 @@ def _to_criterion(self) -> Criterion | LogicalCriterionCombination | None:
                         f"Extension type {extension['type']} not supported yet"
                     )
 
-                drug_action.exclude = False  # reset the exclude flag, as it is now part of the combination
-
                 ext_criterion = PointInTimeCriterion(
-                    exclude=False,  # extensions are always included (at least for now)
                     category=CohortCategory.INTERVENTION,
                     concept=extension["code"],
                     value=extension["value"],
                 )
 
+                # A Conditional Filter returns `right` iff left is POSITIVE, otherwise it returns NEGATIVE
+                # rational: "conditional" extensions are some conditions for dosage, such as body weight ranges.
+                # Thus, the actual drug administration (drug_action, "right") must only be fulfilled if the
+                # condition (ext_criterion, "left") is fulfilled. Thus, we here add this conditional filter.
                 comb = NonCommutativeLogicalCriterionCombination.ConditionalFilter(
-                    exclude=drug_action.exclude,  # need to pull up the exclude flag from the criterion into the combination
                     category=CohortCategory.INTERVENTION,
                     left=ext_criterion,
                     right=drug_action,
                 )
 
                 drug_actions.append(comb)
 
+        result: Criterion | CriterionCombination
         if len(drug_actions) == 1:
-            # set the exclude flag to the value of the action, as this is the only action
-            drug_actions[0].exclude = self._exclude
-            return drug_actions[0]
+            result = drug_actions[0]
         else:
-            comb = LogicalCriterionCombination(
-                exclude=self._exclude,
+            result = LogicalCriterionCombination(
                 category=CohortCategory.INTERVENTION,
                 operator=LogicalCriterionCombination.Operator("OR"),
             )
-            comb.add_all(drug_actions)
-            return comb
+            result.add_all(drug_actions)
+        return result

execution_engine/converter/characteristic/abstract.py

@@ -97,6 +97,11 @@ def get_concept(cc: Coding, standard: bool = True) -> Concept:
         return standard_vocabulary.get_concept(cc.system, cc.code, standard=standard)
 
     @abstractmethod
-    def to_criterion(self) -> Criterion:
-        """Converts this characteristic to a Criterion."""
+    def to_positive_criterion(self) -> Criterion:
+        """
+        Converts this characteristic to a "Positive" Criterion.
+
+        Positive criterion means that a possible excluded flag is disregarded. Instead, the exclusion
+        is later introduced (in the to_criterion() method) via a LogicalCriterionCombination.Not).
+        """
         raise NotImplementedError()

execution_engine/converter/characteristic/codeable_concept.py

@@ -79,10 +79,9 @@ def from_fhir(
 
         return c
 
-    def to_criterion(self) -> Criterion:
+    def to_positive_criterion(self) -> Criterion:
         """Converts this characteristic to a Criterion."""
         return self._criterion_class(
-            exclude=self._exclude,
             category=CohortCategory.POPULATION,
             concept=self.value,
             value=None,

execution_engine/converter/characteristic/value.py

@@ -33,10 +33,9 @@ def from_fhir(
 
         return c
 
-    def to_criterion(self) -> ConceptCriterion:
+    def to_positive_criterion(self) -> ConceptCriterion:
         """Converts this characteristic to a Criterion."""
         return self._criterion_class(
-            exclude=self._exclude,
             category=CohortCategory.POPULATION,
             concept=self.type,
             value=self.value,

execution_engine/converter/converter.py

@@ -112,7 +112,9 @@ class CriterionConverter(ABC):
     """
 
     def __init__(self, exclude: bool):
-        # todo: is exclude still required?
+        # The _exclude attribute is used in converter classes but gets
+        # turned into a LogicalCriterionCombination with operator NOT
+        # in the to_criterion method.
         self._exclude = exclude
 
     @classmethod
@@ -130,10 +132,25 @@ def valid(cls, fhir_definition: Element) -> bool:
         raise NotImplementedError()
 
     @abstractmethod
-    def to_criterion(self) -> Criterion | LogicalCriterionCombination:
-        """Converts this characteristic to a Criterion."""
+    def to_positive_criterion(self) -> Criterion | LogicalCriterionCombination:
+        """Converts this characteristic to a Criterion or a combination of criteria but no negation."""
         raise NotImplementedError()
 
+    def to_criterion(self) -> Criterion | LogicalCriterionCombination:
+        """
+        Converts this characteristic to a Criterion or a
+        combination of criteria. The result may be a "negative"
+        criterion, that is the result of to_positive_criterion wrapped
+        in a LogicalCriterionCombination with operator NOT.
+        """
+        positive_criterion = self.to_positive_criterion()
+        if self._exclude:
+            return LogicalCriterionCombination.Not(
+                positive_criterion, positive_criterion.category
+            )
+        else:
+            return positive_criterion
+
 
 class CriterionConverterFactory:
     """Factory for creating a new instance of a criterion converter."""

execution_engine/converter/goal/assessment_scale.py

@@ -47,12 +47,11 @@ def from_fhir(cls, goal: PlanDefinitionGoal) -> "AssessmentScaleGoal":
 
         return cls(code.concept_name, exclude=False, code=code, value=value)
 
-    def to_criterion(self) -> Criterion:
+    def to_positive_criterion(self) -> Criterion:
         """
         Converts the goal to a criterion.
         """
         return Measurement(
-            exclude=self._exclude,
             category=CohortCategory.INTERVENTION,
             concept=self._code,
             value=self._value,

execution_engine/converter/goal/laboratory_value.py

@@ -46,12 +46,11 @@ def from_fhir(cls, goal: PlanDefinitionGoal) -> "LaboratoryValueGoal":
 
         return cls(exclude=False, code=code, value=value)
 
-    def to_criterion(self) -> Criterion:
+    def to_positive_criterion(self) -> Criterion:
         """
         Converts the goal to a criterion.
         """
         return Measurement(
-            exclude=self._exclude,
             category=CohortCategory.INTERVENTION,
             concept=self._code,
             value=self._value,

execution_engine/converter/goal/ventilator_management.py

@@ -59,21 +59,19 @@ def from_fhir(cls, goal: PlanDefinitionGoal) -> "VentilatorManagementGoal":
 
         return cls(exclude=False, code=code, value=value)
 
-    def to_criterion(self) -> Criterion:
+    def to_positive_criterion(self) -> Criterion:
         """
         Converts the goal to a criterion.
         """
         if self._code in CUSTOM_GOALS:
             cls = CUSTOM_GOALS[self._code]
             return cls(
-                exclude=False,
                 category=CohortCategory.INTERVENTION,
                 concept=self._code,
                 value=self._value,
             )
 
         return Measurement(
-            exclude=self._exclude,
             category=CohortCategory.INTERVENTION,
             concept=self._code,
             value=self._value,

execution_engine/converter/parser/fhir_parser_v1.py

@@ -176,6 +176,5 @@ def parse_action_combination_method(
             )
         return LogicalCriterionCombination(
             category=CohortCategory.INTERVENTION,
-            exclude=False,
             operator=operator,
         )

execution_engine/converter/parser/fhir_parser_v2.py

@@ -66,6 +66,5 @@ def parse_action_combination_method(
 
         return LogicalCriterionCombination(
             category=CohortCategory.INTERVENTION,
-            exclude=False,
             operator=operator,
         )

execution_engine/execution_graph/graph.py

@@ -1,4 +1,4 @@
-from typing import Any, Callable, Type, cast
+from typing import Any, Callable, Type
 
 import networkx as nx
 
@@ -378,7 +378,7 @@ def conjunction_from_combination(
             comb: CriterionCombination,
         ) -> Type[logic.BooleanFunction] | Callable:
             """
-            Convert the criterion's operator into a logical conjunction (And or Or)
+            Convert the criterion's operator into a logical conjunction (Not or And or Or)
             """
             if isinstance(comb, LogicalCriterionCombination):
                 if comb.is_root():
@@ -397,6 +397,8 @@ def conjunction_from_combination(
                     return logic.NonSimplifiableAnd
                 elif isinstance(comb, NonCommutativeLogicalCriterionCombination):
                     return logic.ConditionalFilter
+                elif comb.operator.operator == LogicalCriterionCombination.Operator.NOT:
+                    return logic.Not
                 elif comb.operator.operator == LogicalCriterionCombination.Operator.AND:
                     return logic.And
                 elif comb.operator.operator == LogicalCriterionCombination.Operator.OR:
@@ -513,24 +515,11 @@ def _traverse(comb: CriterionCombination) -> logic.Expr:
                 if isinstance(entry, CriterionCombination):
                     components.append(_traverse(entry))
                 elif isinstance(entry, Criterion):
-                    # Remove the exclude criterion from the symbol, as it is handled by the Not operator
-                    s = logic.Symbol(
-                        criterion=cast(Criterion, entry.clear_exclude(inplace=False))
-                    )
-
-                    if entry.exclude:
-                        s = logic.Not(s, category=entry.category)
-
-                    components.append(s)
+                    components.append(logic.Symbol(entry))
                 else:
                     raise ValueError(f"Invalid entry type: {type(entry)}")
 
-            c = conjunction(*components, category=comb.category)
-
-            if comb.exclude:
-                c = logic.Not(c, category=comb.category)
-
-            return c
+            return conjunction(*components, category=comb.category)
 
         expression = _traverse(comb)
 

execution_engine/fhir_omop_mapping.py

@@ -38,11 +38,14 @@ def characteristic_to_criterion(
         )
         comb = LogicalCriterionCombination(
             category=CohortCategory.POPULATION,
-            exclude=characteristic.exclude,
             operator=operator,
         )
         for c in characteristic:
             comb.add(characteristic_to_criterion(c))
+
+        if characteristic.exclude:
+            comb = LogicalCriterionCombination.Not(comb, CohortCategory.POPULATION)
+
         return comb
     else:
         return characteristic.to_criterion()

execution_engine/omop/cohort/population_intervention_pair.py

@@ -69,7 +69,6 @@ def set_criteria(
         """
 
         root_combination = LogicalCriterionCombination(
-            exclude=False,
             category=category,
             operator=LogicalCriterionCombination.Operator("AND"),
             root_combination=True,
@@ -168,7 +167,7 @@ def _assert_base_table_in_select(
         """
         Assert that the base table is used in the select statement.
 
-        Joining the base table ensures that always just a subset of potients are selected,
+        Joining the base table ensures that always just a subset of patients is selected,
         not all.
         """
         if isinstance(sql, SelectInto) or isinstance(sql, Insert):

execution_engine/omop/cohort/recommendation.py

@@ -164,7 +164,6 @@ def criteria(self) -> CriterionCombination:
         Get the criteria of the recommendation.
         """
         criteria = LogicalCriterionCombination(
-            exclude=False,
             category=CohortCategory.BASE,
             operator=LogicalCriterionCombination.Operator("OR"),
             root_combination=True,