Closed jenniferduryea closed 6 years ago
@jeremyevans has done some updates to the operator - where he added the fields (in screenshot):
His commits, with notes, are noted here: https://github.com/outcomesinsights/conceptql/pull/163
For documentation:
Ran a test on broom.jsaw.io and found the export failed with the following error:
ERROR -- : Impala::Protocol::Beeswax::BeeswaxException: AnalysisException: Column/field reference is ambiguous: 'person_id'
Sending back to @aguynamedryan for help.
Per @aguynamedryan request, here is the JSON for the above test case:
@jenniferduryea, could you please cut and paste the text itself so we don't have to re-type the algorithm?
@aguynamedryan here you go:
[["occurrence",4,["icd9","355.8","401.0","401.9","250.00","780.93","459.81"],{"within":"60d"}]]
My test patient (person_id = 2) is not showing up in my test I posted above when run against synpuf_250. I looked at the output and it looks like this is not including all results. Tagging @jeremyevans for help.
My original test (find fourth dxs within 60d of each other will result with patient 2, index 4/15/09) passed.
However, I'm now testing the at_least parameter (find 2nd dxs at_least 30d will result in patient 2 with 3/30/09 event) and I'm getting 0 results. I ran the SQL code through HUE (against synpuf_250 directly) and got 0 results. This seems like a bug. Sending back to @jeremyevans for help.
For reference, here is the conceptql code I'm using: [["occurrence",2,["icd9","355.8","401.0","401.9","250.00","780.93","459.81"],{"within":"","at_least":"30d"}]]
The test should include patient 2 with an event at 3/30/09 since the first event (10/4/08) is greater than 30d apart.
I have updated the original ticket description to document how we think the nth occurrence operator is working. @jeremyevans please review the original ticket description about AT_LEAST to make sure we are all on the same page. Thank you.
I think the issue is that empty within or at_least options are not handled the way you expect. Looks like you want to ignore the value if it is empty. I think the pull request just referenced will fix the issue.
Tested using all parameters (at_least, within, occurrences, group_by_day) and they all work as expected. Thank you so much @jeremyevans !
The following tests were created to test the functionality of this operator: nth_occurrence_test nth_occurrence_test_as_2out nth_occurrence_test_as_2out_2d_atleast_30d_within nth_occurrence_test_as_2out_8th_within_365d_group_by_day nth_occurrence_test_as_2out_190d
Taking the use case from #160 , instead of adding that feature into the AFTER operator, it seems better fit to add this functionality into the Nth Occurrence operator. Please view #160 for the actual use case and what is correct behavior. In summary, the use case is trying to find the 4th event within a time frame, and have that time frame reoccur or "roll" along - similar to recurrent event studies.
For documentation purposes, I'm going to explain how this operator works (in summary).
The nth occurrence operator will find the nth occurrence, which will return one qualifying event per parameter. The occurrence count starts at using the reference/first event as "1".