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

Support for datetime calculations in cohort definitions #200

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1fa1d69
Support for datetime calculations in cohort definitions
jcnamendiOdysseus Dec 18, 2023
b9b5820
update
jcnamendiOdysseus Dec 29, 2023
6fd8ecd
update/412023
jcnamendiOdysseus Jan 4, 2024
07a7160
add sql test
jcnamendiOdysseus Jan 8, 2024
eaf44d5
fix
jcnamendiOdysseus Jan 9, 2024
6dc3918
Merge remote-tracking branch 'origin/is/2886' into issues-2886
jcnamendiOdysseus Jan 9, 2024
603c800
update test
jcnamendiOdysseus Jan 9, 2024
fe4f7e5
fix indent on editor
jcnamendiOdysseus Jan 9, 2024
75a2906
Merge pull request #6 from OHDSI/master
alex-odysseus Jan 15, 2024
3210fae
Merge remote-tracking branch 'remotes/origin/master' into issues-2886
alex-odysseus Jan 15, 2024
e7f90a6
remove jar
jcnamendiOdysseus Jan 15, 2024
0036dc0
Merge remote-tracking branch 'origin/issues-2886' into issues-2886
jcnamendiOdysseus Jan 15, 2024
4d0cb33
Support for datetime calculations in cohort definitions
jcnamendiOdysseus Dec 18, 2023
55b2eef
update
jcnamendiOdysseus Dec 29, 2023
806c85b
update/412023
jcnamendiOdysseus Jan 4, 2024
8a4eb23
add sql test
jcnamendiOdysseus Jan 8, 2024
f819d5e
fix
jcnamendiOdysseus Jan 9, 2024
5e03963
update test
jcnamendiOdysseus Jan 9, 2024
de9527c
fix indent on editor
jcnamendiOdysseus Jan 9, 2024
ca6bbfd
remove jar
jcnamendiOdysseus Jan 15, 2024
cebb8c5
Revert "fix"
jcnamendiOdysseus Jan 16, 2024
2ada34d
Merge remote-tracking branch 'origin/issues-2886' into issues-2886
jcnamendiOdysseus Jan 17, 2024
6fb7fd2
add tests
jcnamendiOdysseus Jan 17, 2024
da13f6e
Adding IntervalUnit to Criteria to specify datetime table columns whi…
alex-odysseus Jan 23, 2024
690b273
Replace wildcard import with specific imports
jcnamendiOdysseus Jan 23, 2024
2f97201
Enhance time unit handling in compareTo() method
jcnamendiOdysseus Jan 25, 2024
4209ca0
Introduce time interval testing functionality and enhance the impleme…
jcnamendiOdysseus Jan 25, 2024
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
Prev Previous commit
Next Next commit
update
  • Loading branch information
jcnamendiOdysseus committed Jan 16, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 55b2eefeaf5d76ddbba48eba5f7892688d9fbd60
45 changes: 22 additions & 23 deletions src/main/java/org/ohdsi/circe/check/checkers/Comparisons.java
Original file line number Diff line number Diff line change
@@ -20,9 +20,7 @@

import java.time.LocalDate;
import java.time.format.DateTimeParseException;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.*;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.apache.commons.lang3.builder.EqualsBuilder;
@@ -31,6 +29,12 @@

public class Comparisons {

private static final Map<String, Integer> TIME_UNIT_CONVERSION = new HashMap<>();
static {
TIME_UNIT_CONVERSION.put(IntervalUnit.HOUR.getName(), 60 * 60);
TIME_UNIT_CONVERSION.put(IntervalUnit.MINUTE.getName(), 60);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't do it this way, it makes it seem like it's dynamic (ie: can change at runtime). Can't this function of conversion be associated to the enum itself?

public static Boolean startIsGreaterThanEnd(NumericRange r) {

return Objects.nonNull(r.value) && Objects.nonNull(r.extent) && r.value.intValue() > r.extent.intValue();
@@ -101,31 +105,26 @@ public static Predicate<Concept> compare(Concept source) {
.append(concept.vocabularyId, source.vocabularyId)
.build();
}

public static int compareTo(ObservationFilter filter, Window window) {
int range1 = (filter.postDays + filter.priorDays) * 24 * 60 * 60;
int range2Start = getTimeInSeconds(window.start);
int range2End = getTimeInSeconds(window.end);
int range1 , range2Start , range2End;
if(Objects.nonNull(window.start) && Objects.nonNull(window.start.days)){
range1 = filter.postDays + filter.priorDays;
range2Start = window.start.coeff * window.start.days;
range2End = Objects.nonNull(window.end) && Objects.nonNull(window.end.days) ? window.end.coeff * window.end.days : 0;
}else {
range1 = (filter.postDays + filter.priorDays) * 24 * 60 * 60;
range2Start = getTimeInSeconds(window.start);
range2End = getTimeInSeconds(window.end);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is where the 4sp indent is glaring, so before you have to go back and change all of it back to 2sp, i'd adjust your formatting.

}
return range1 - (range2End - range2Start);
}

private static int getTimeInSeconds(Window.Endpoint endpoint) {
if (Objects.isNull(endpoint)) {
return 0;
return Optional.ofNullable(endpoint)
.map(ep -> {
int convertRate = TIME_UNIT_CONVERSION.getOrDefault(ep.timeUnit, 1);
return Objects.nonNull(ep.timeUnitValue) ? ep.coeff * ep.timeUnitValue * convertRate : 0;
}).orElse(0);
}
int convertRate;
if (endpoint.timeUnit.equals(IntervalUnit.DAY.getName())) {
convertRate = 24 * 60 * 60;
} else if (endpoint.timeUnit.equals(IntervalUnit.HOUR.getName())) {
convertRate = 60 * 60;
} else if (endpoint.timeUnit.equals(IntervalUnit.MINUTE.getName())) {
convertRate = 60;
} else convertRate = 1;
return Objects.nonNull(endpoint.timeUnitValue)
? endpoint.coeff * endpoint.timeUnitValue * convertRate
: 0;
}


public static boolean compare(Criteria c1, Criteria c2) {

Original file line number Diff line number Diff line change
@@ -18,14 +18,17 @@

package org.ohdsi.circe.check.checkers;

import static org.ohdsi.circe.check.operations.Operations.match;
import static org.ohdsi.circe.cohortdefinition.DateOffsetStrategy.DateField.StartDate;

import java.util.Objects;
import org.ohdsi.circe.check.WarningSeverity;
import org.ohdsi.circe.cohortdefinition.CohortExpression;
import org.ohdsi.circe.cohortdefinition.DateOffsetStrategy;

import java.util.Arrays;
import java.util.List;
import java.util.Objects;

import static org.ohdsi.circe.check.operations.Operations.match;
import static org.ohdsi.circe.cohortdefinition.DateOffsetStrategy.DateField.StartDate;

public class ExitCriteriaDaysOffsetCheck extends BaseCheck {

private static final String DAYS_OFFSET_WARNING = "Cohort Exit criteria: %ss offset from start date should be greater than 0";
@@ -40,9 +43,15 @@ protected WarningSeverity defineSeverity() {
protected void check(CohortExpression expression, WarningReporter reporter) {

match(expression.endStrategy)
.isA(DateOffsetStrategy.class)
.then(s -> match((DateOffsetStrategy)s)
.when(dateOffsetStrategy -> Objects.equals(StartDate, dateOffsetStrategy.dateField) && 0 == dateOffsetStrategy.offsetUnitValue)
.then(dateOffsetStrategy -> reporter.add(String.format(DAYS_OFFSET_WARNING, dateOffsetStrategy.offsetUnit))));
.isA(DateOffsetStrategy.class)
.then(s -> match((DateOffsetStrategy)s)
.when(dateOffsetStrategy -> Objects.equals(StartDate, dateOffsetStrategy.dateField) && (0 == dateOffsetStrategy.offset || 0 == dateOffsetStrategy.offsetUnitValue))
.then(dateOffsetStrategy -> {
if(0 == dateOffsetStrategy.offset){
reporter.add(String.format(DAYS_OFFSET_WARNING), "Days");
}else {
reporter.add(String.format(DAYS_OFFSET_WARNING, dateOffsetStrategy.offsetUnit));
}
}));
}
}
Original file line number Diff line number Diff line change
@@ -43,8 +43,11 @@ protected void checkCriteria(CorelatedCriteria criteria, String groupName, Warni
Execution addWarning = () -> reporter.add(WARNING, name);
match(criteria)
.when(c -> c.startWindow != null && ((c.startWindow.start != null
&& c.startWindow.start.timeUnitValue != null) || (c.startWindow.end != null
&& c.startWindow.end.timeUnitValue != null)))
&& c.startWindow.start.days != null) || (c.startWindow.end != null
&& c.startWindow.end.days != null))
|| ((c.startWindow.start.timeUnitValue != null)
&& (c.startWindow.end != null)
&& c.startWindow.end.timeUnitValue != null))
.then(cc -> match(cc.criteria)
.isA(ConditionEra.class)
.then(c -> match((ConditionEra)c)
26 changes: 18 additions & 8 deletions src/main/java/org/ohdsi/circe/check/checkers/RangeCheck.java
Original file line number Diff line number Diff line change
@@ -25,6 +25,7 @@
import org.ohdsi.circe.cohortdefinition.Window;

import java.util.Objects;
import java.util.Optional;

public class RangeCheck extends BaseValueCheck {
private static final String NEGATIVE_VALUE_ERROR = "Time window in criteria \"%s\" has negative value %d at %s";
@@ -52,15 +53,24 @@ protected void checkInclusionRules(final CohortExpression expression, WarningRep
}

private void checkWindow(Window window, WarningReporter reporter, String name) {
Optional.ofNullable(window)
.map(w -> checkEndpoint(w, name, "start"))
.ifPresent(reporter::add);

if (Objects.nonNull(window)) {
if (Objects.nonNull(window.start) && Objects.nonNull(window.start.timeUnitValue) && window.start.timeUnitValue < 0) {
reporter.add(NEGATIVE_VALUE_ERROR, name, window.start.timeUnitValue, "start");
}
if (Objects.nonNull(window.end) && Objects.nonNull(window.end.timeUnitValue) && window.end.timeUnitValue < 0) {
reporter.add(NEGATIVE_VALUE_ERROR, name, window.end.timeUnitValue, "end");
}
}
Optional.ofNullable(window)
.map(w -> checkEndpoint(w, name, "end"))
.ifPresent(reporter::add);
}

private String checkEndpoint(Window window, String name, String endpointType) {
boolean hasValid = Objects.nonNull(window.start) && Objects.nonNull(window.start.days < 0 ? window.start.days : window.start.timeUnitValue) && window.start.days < 0 ? window.start.days < 0 : window.start.timeUnitValue < 0;
return Optional.of(window)
.filter(w -> hasValid)
.map(w -> String.format(NEGATIVE_VALUE_ERROR, name, getEndpointValue(w.start), endpointType))
.orElse(null);
}
private Object getEndpointValue(Window.Endpoint endpoint) {
return Objects.nonNull(endpoint.days) ? endpoint.days : endpoint.timeUnitValue;
}

private void checkObservationFilter(ObservationFilter filter, WarningReporter reporter, String name) {
25 changes: 17 additions & 8 deletions src/main/java/org/ohdsi/circe/check/checkers/TimePatternCheck.java
Original file line number Diff line number Diff line change
@@ -18,10 +18,7 @@

package org.ohdsi.circe.check.checkers;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.*;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.ohdsi.circe.check.WarningSeverity;
@@ -83,17 +80,29 @@ private String formatTimeWindow(TimeWindowInfo ti) {
}

private String formatDays(Window.Endpoint endpoint) {
return Objects.nonNull(endpoint.timeUnitValue) ? String.valueOf(endpoint.timeUnitValue) : "all";
return Objects.nonNull(endpoint.days) ? String.valueOf(endpoint.days) :
Objects.nonNull(endpoint.timeUnitValue) ? String.valueOf(endpoint.timeUnitValue) : "all";

}

private String formatCoeff(Window.Endpoint endpoint) {
return endpoint.coeff < 0 ? "before " : "after ";
}


private Integer startDays(Window window) {
return Objects.nonNull(window) && Objects.nonNull(window.start) ?
(Objects.nonNull(window.start.timeUnitValue) ? window.start.timeUnitValue : 0) * window.start.coeff : 0;
}
return Optional.ofNullable(window)
.map(w -> w.start)
.map(start -> {
int coefficient = Optional.ofNullable(start.coeff).orElse(0);
return Optional.ofNullable(start.days)
.map(days -> days * coefficient)
.orElseGet(() -> Optional.ofNullable(start.timeUnitValue)
.map(timeUnitValue -> timeUnitValue * coefficient)
.orElse(0));
})
.orElse(0);
}

class TimeWindowInfo {
private String name;
Original file line number Diff line number Diff line change
@@ -20,9 +20,8 @@

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import org.apache.commons.lang3.StringUtils;
@@ -351,10 +350,12 @@ public String buildExpressionQuery(CohortExpression expression, BuildExpressionQ

resultSql = StringUtils.replace(resultSql, "@cohort_end_unions", StringUtils.join(endDateSelects, "\nUNION ALL\n"));

// resultSql = StringUtils.replace(resultSql, "@eraconstructorpad", Integer.toString(expression.collapseSettings.eraPad));
resultSql = StringUtils.replace(resultSql, "@era_pad_unit", expression.collapseSettings.eraPadUnit);
resultSql = StringUtils.replace(resultSql, "@era_pad", Integer.toString(expression.collapseSettings.eraPadUnitValue));

if(!StringUtils.isEmpty(Integer.toString(expression.collapseSettings.eraPad)) && expression.collapseSettings.eraPad != 0){
resultSql = StringUtils.replace(resultSql, "@eraconstructorpad", Integer.toString(expression.collapseSettings.eraPad));
}else{
resultSql = StringUtils.replace(resultSql, "@era_pad_unit", expression.collapseSettings.eraPadUnit);
resultSql = StringUtils.replace(resultSql, "@era_pad", Integer.toString(expression.collapseSettings.eraPadUnitValue));
}
resultSql = StringUtils.replace(resultSql, "@inclusionRuleTable", getInclusionRuleTableSql(expression));
resultSql = StringUtils.replace(resultSql, "@inclusionImpactAnalysisByEventQuery", getInclusionAnalysisQuery("#qualified_events", 0));
resultSql = StringUtils.replace(resultSql, "@inclusionImpactAnalysisByPersonQuery", getInclusionAnalysisQuery("#best_events", 1));
@@ -548,20 +549,25 @@ public String getWindowedCriteriaQuery(String sqlTemplate, WindowedCriteria crit
Window startWindow = criteria.startWindow;
String startIndexDateExpression = (startWindow.useIndexEnd != null && startWindow.useIndexEnd) ? "P.END_DATE" : "P.START_DATE";
String startEventDateExpression = (startWindow.useEventEnd != null && startWindow.useEventEnd) ? "A.END_DATE" : "A.START_DATE";
if (startWindow.start.timeUnitValue != null) {
startExpression = String.format("DATEADD(%s,%d,%s)", startWindow.start.timeUnit, startWindow.start.coeff * startWindow.start.timeUnitValue, startIndexDateExpression);
if (startWindow.start.days != null) {
startExpression = String.format("DATEADD(day,%d,%s)", startWindow.start.coeff * startWindow.start.days, startIndexDateExpression);
} else if (startWindow.start.timeUnitValue != null) {
startExpression = String.format("DATEADD(%s,%d,%s)", startWindow.start.timeUnit, startWindow.start.coeff * startWindow.start.timeUnitValue, startIndexDateExpression);
} else {
startExpression = checkObservationPeriod ? (startWindow.start.coeff == -1 ? "P.OP_START_DATE" : "P.OP_END_DATE") : null;
startExpression = checkObservationPeriod ? (startWindow.start.coeff == -1 ? "P.OP_START_DATE" : "P.OP_END_DATE") : null;
}

if (startExpression != null) {
clauses.add(String.format("%s >= %s", startEventDateExpression, startExpression));
}

if (startWindow.end.timeUnitValue != null) {
endExpression = String.format("DATEADD(%s,%d,%s)", startWindow.start.timeUnit, startWindow.end.coeff * startWindow.end.timeUnitValue, startIndexDateExpression);
if (startWindow.end.days != null) {
endExpression = String.format("DATEADD(day,%d,%s)", startWindow.end.coeff * startWindow.end.days, startIndexDateExpression);
}
else if (startWindow.end.timeUnitValue != null) {
endExpression = String.format("DATEADD(%s,%d,%s)", startWindow.start.timeUnit, startWindow.end.coeff * startWindow.end.timeUnitValue, startIndexDateExpression);
} else {
endExpression = checkObservationPeriod ? (startWindow.end.coeff == -1 ? "P.OP_START_DATE" : "P.OP_END_DATE") : null;
endExpression = checkObservationPeriod ? (startWindow.end.coeff == -1 ? "P.OP_START_DATE" : "P.OP_END_DATE") : null;
}

if (endExpression != null) {
@@ -575,20 +581,24 @@ public String getWindowedCriteriaQuery(String sqlTemplate, WindowedCriteria crit
String endIndexDateExpression = (endWindow.useIndexEnd != null && endWindow.useIndexEnd) ? "P.END_DATE" : "P.START_DATE";
// for backwards compatability, having a null endWindow.useIndexEnd means they SHOULD use the index end date.
String endEventDateExpression = (endWindow.useEventEnd == null || endWindow.useEventEnd) ? "A.END_DATE" : "A.START_DATE";
if (endWindow.start.timeUnitValue != null) {
startExpression = String.format("DATEADD(%s,%d,%s)", endWindow.start.timeUnit, endWindow.start.coeff * endWindow.start.timeUnitValue, endIndexDateExpression);
if (endWindow.start.days != null) {
startExpression = String.format("DATEADD(day,%d,%s)", endWindow.start.coeff * endWindow.start.days, endIndexDateExpression);
} else if (endWindow.start.timeUnitValue != null) {
startExpression = String.format("DATEADD(%s,%d,%s)", endWindow.start.timeUnit, endWindow.start.coeff * endWindow.start.timeUnitValue, endIndexDateExpression);
} else {
startExpression = checkObservationPeriod ? (endWindow.start.coeff == -1 ? "P.OP_START_DATE" : "P.OP_END_DATE") : null;
startExpression = checkObservationPeriod ? (endWindow.start.coeff == -1 ? "P.OP_START_DATE" : "P.OP_END_DATE") : null;
}

if (startExpression != null) {
clauses.add(String.format("%s >= %s", endEventDateExpression, startExpression));
}

if (endWindow.end.timeUnitValue != null) {
endExpression = String.format("DATEADD(%s,%d,%s)", endWindow.start.timeUnit, endWindow.end.coeff * endWindow.end.timeUnitValue, endIndexDateExpression);
if (endWindow.end.days != null) {
endExpression = String.format("DATEADD(day,%d,%s)", endWindow.end.coeff * endWindow.end.days, endIndexDateExpression);
} else if (endWindow.end.timeUnitValue != null) {
endExpression = String.format("DATEADD(%s,%d,%s)", endWindow.start.timeUnit, endWindow.end.coeff * endWindow.end.timeUnitValue, endIndexDateExpression);
} else {
endExpression = checkObservationPeriod ? (endWindow.end.coeff == -1 ? "P.OP_START_DATE" : "P.OP_END_DATE") : null;
endExpression = checkObservationPeriod ? (endWindow.end.coeff == -1 ? "P.OP_START_DATE" : "P.OP_END_DATE") : null;
}

if (endExpression != null) {
@@ -765,10 +775,13 @@ private String getDateFieldForOffsetStrategy(DateOffsetStrategy.DateField dateFi
@Override
public String getStrategySql(DateOffsetStrategy strat, String eventTable) {
String strategySql = StringUtils.replace(DATE_OFFSET_STRATEGY_TEMPLATE, "@eventTable", eventTable);
strategySql = StringUtils.replace(strategySql, "@offsetUnitValue", Integer.toString(strat.offsetUnitValue));
strategySql = StringUtils.replace(strategySql, "@offsetUnit", strat.offsetUnit);
if(strat.offset != 0){
strategySql = StringUtils.replace(strategySql, "@offset", Integer.toString(strat.offset));
}else {
strategySql = StringUtils.replace(strategySql, "@offsetUnitValue", Integer.toString(strat.offsetUnitValue));
strategySql = StringUtils.replace(strategySql, "@offsetUnit", strat.offsetUnit);
}
strategySql = StringUtils.replace(strategySql, "@dateField", getDateFieldForOffsetStrategy(strat.dateField));

return strategySql;
}

@@ -793,4 +806,5 @@ public String getStrategySql(CustomEraStrategy strat, String eventTable) {
}

// </editor-fold>

}
Original file line number Diff line number Diff line change
@@ -24,8 +24,10 @@ public class CollapseSettings {

@JsonProperty("CollapseType")
public CollapseType collapseType = CollapseType.ERA;
@JsonProperty("EraPad")
public int eraPad = 0;
@JsonProperty("EraPadUnit")
public String eraPadUnit = IntervalUnit.DAY.getName();
public String eraPadUnit;
@JsonProperty("EraPadUnitValue")
public int eraPadUnitValue = 0;

Original file line number Diff line number Diff line change
@@ -30,14 +30,15 @@ public enum DateField {
StartDate,
EndDate
}

@JsonProperty("Offset")
public int offset = 0;
@JsonProperty("DateField")
public DateField dateField = DateField.StartDate;

@JsonProperty("OffsetUnitValue")
public int offsetUnitValue = 0;
@JsonProperty("OffsetUnit")
public String offsetUnit = IntervalUnit.DAY.getName();
public String offsetUnit;

@Override
public String accept(IGetEndStrategySqlDispatcher dispatcher, String eventTable) {
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@
import jdk.nashorn.internal.objects.annotations.Getter;

public enum IntervalUnit {
DAY("day"),
HOUR("hour"),
MINUTE("minute"),
SECOND("second");
Original file line number Diff line number Diff line change
@@ -4,12 +4,9 @@
import org.ohdsi.circe.cohortdefinition.ConditionOccurrence;
import org.ohdsi.circe.helper.ResourceHelper;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.text.SimpleDateFormat;
import java.util.*;

import org.ohdsi.circe.cohortdefinition.DateAdjustment;

import static org.ohdsi.circe.cohortdefinition.builders.BuilderUtils.buildDateRangeClause;
@@ -65,7 +62,6 @@ protected String embedCodesetClause(String query, T criteria) {

@Override
protected String embedOrdinalExpression(String query, T criteria, List<String> whereClauses) {

// first
if (criteria.first != null && criteria.first == true) {
whereClauses.add("C.ordinal = 1");
Loading