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.

Sign up for GitHub

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

Fix criteria offsets #19

Merged
merged 3 commits into from
Oct 1, 2024
Merged

Fix criteria offsets #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
daba2df
Update ckd_post_kidney_labs.yaml
justin13601 Oct 1, 2024
13b1083
Update 1y-5y.yaml
justin13601 Oct 1, 2024
9aee535
Update 1y-5y.yaml
justin13601 Oct 1, 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
2 changes: 1 addition & 1 deletion src/MEDS_DEV/tasks/criteria/disease_progression/diabetes/ckd_post_kidney_labs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ windows:
index_timestamp: end
target:
start: input.end
end: start + 5y
end: start + 1825D # 5 years
start_inclusive: False
end_inclusive: True
label: ckd_dx
Expand Down
6 changes: 3 additions & 3 deletions src/MEDS_DEV/tasks/criteria/phenotyping/outpatient/MI/1y-5y.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ windows:
index_timestamp: end
gap:
start: trigger
end: trigger + 1y
end: start + 365D
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent Time Window Definitions After Conversion

The change from trigger + 1y to start + 365D in src/MEDS_DEV/tasks/criteria/phenotyping/outpatient/MI/1y-5y.yaml results in:

  • start: trigger + 365D
  • end: start + 365D (effectively trigger + 730D)
  • end: start + 1825D (# 5 years)

This adjustment shifts the gap window from one year after the trigger to two years, which may deviate from the intended criteria of "within 1 year of their annual physical."

🔗 Analysis chain

Approve the conversion to days, but verify the use of 'start'

The change from trigger + 1y to start + 365D aligns with the PR objective of expressing years in terms of days, which improves precision. However, using start instead of trigger might alter the semantic meaning of the gap window.

Please confirm if start + 365D is semantically equivalent to the previous trigger + 1y. If not, consider using trigger + 365D to maintain consistency with the description that mentions "within 1 year of their annual physical".

To verify the impact of this change, you can run the following script:

This will help ensure consistency across similar task definitions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other occurrences of time windows defined relative to 'trigger' or 'start'

# Search for 'trigger' and 'start' in time window definitions
rg --type yaml 'trigger \+|\bstart \+' src/MEDS_DEV/tasks/criteria/phenotyping/

Length of output: 357

Script:

#!/bin/bash
# Description: Verify consistency of time window definitions across all phenotyping outpatient MI YAML files

# Search for YAML files in src/MEDS_DEV/tasks/criteria/phenotyping/outpatient/MI/ directory
fd --type f --extension yaml src/MEDS_DEV/tasks/criteria/phenotyping/outpatient/MI/ | while read file; do
  echo "File: $file"
  rg 'start\s*:\s*trigger\s*\+\s*\d+D' "$file"
  rg 'end\s*:\s*start\s*\+\s*\d+D' "$file"
done

Length of output: 619

Script:

#!/bin/bash
# Description: Verify consistency of time window definitions across all phenotyping outpatient MI YAML files

# Search for YAML files in src/MEDS_DEV/tasks/criteria/phenotyping/outpatient/MI/ directory using a match-all pattern
fd . --type f --extension yaml --full-path 'src/MEDS_DEV/tasks/criteria/phenotyping/outpatient/MI/' | while read file; do
  echo "File: $file"
  rg 'start\s*:\s*trigger\s*\+\s*\d+D' "$file"
  rg 'end\s*:\s*start\s*\+\s*\d+D' "$file"
done

Length of output: 559

start_inclusive: True
end_inclusive: True
has:
MI: (None, 0)
target:
start: trigger + 1y
end: trigger + 5y
start: trigger + 365D
end: start + 1825D # 5 years
Comment on lines +31 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistency Found in Time Window Definitions

The end time is currently set to start + 1825D, which results in a total duration of approximately 6 years instead of the intended 5 years.

  • File: src/MEDS_DEV/tasks/criteria/phenotyping/outpatient/MI/1y-5y.yaml
    • start: trigger + 365D
    • end: start + 1825D # 5 years
🔗 Analysis chain

Approve the conversion to days, but verify the use of 'start' for the end time

The changes align with the PR objective of expressing years in terms of days, which improves precision. The added comment clarifying that 1825D represents 5 years enhances readability.

Please confirm if start + 1825D is semantically equivalent to the previous trigger + 5y. If not, consider using trigger + 1825D to maintain consistency with the description that mentions "between 1 and 5 years after their annual physical".

To verify the impact of this change and ensure consistency, you can run the following script:

This will help ensure consistency across similar task definitions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other occurrences of time windows defined relative to 'trigger' or 'start'

# Search for 'trigger' and 'start' in time window definitions
rg --type yaml 'trigger \+|\bstart \+' src/MEDS_DEV/tasks/criteria/phenotyping/

Length of output: 357

start_inclusive: False
end_inclusive: True
label: MI
Expand Down
Loading