Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#3357 closed enhancement (fixed)

Query by drug-drug interaction alert description (BPA)

Reported by: rwaitman Owned by: ssuman
Priority: major Milestone: heron-jamestown-update
Component: data-repository Keywords: public-web
Cc: badagarla, mhoag, ngraham, dconnolly Blocked By:
Blocking: 3505, 3536 Sensitive: no

Description

Use specific drug-drug interaction case and example tree organization based on ticket:578#comment:11:

  • How many people had a NSAID/SSRI interaction alert fired? Were the drugs still given afterwards?

Tree:

  • Alerts
    • drug-drug interaction alert
      • alpha folder (A, B...) based on first letter of alert description
        • alert description

Change History (34)

comment:1 Changed 2 years ago by dconnolly

adding these to the heron-weekly agenda per HeronLoad#milestone-close process:

  • Discuss any (major) open tickets ...
  • Review schedule slips: what were the causes? are there cost-effective ways to address those causes?

comment:2 Changed 2 years ago by dconnolly

  • Milestone changed from heron-mcmillan-marsh-update to heron-cheyenne-bottoms-update

Ticket retargeted after milestone closed

comment:3 Changed 2 years ago by dconnolly

  • Keywords heron-weekly added

comment:4 Changed 2 years ago by ngraham

  • Blocking set to 3394

comment:5 Changed 2 years ago by ngraham

  • Keywords heron-weekly removed
  • Milestone changed from heron-cheyenne-bottoms-update to heron-quivira-update

heron-weekly:
SS notes at risk due to clarity 2014 upgrade and EA priorities.
SS: Russ is aware of this and thinks it would be a nice addition.

comment:6 Changed 2 years ago by ngraham

  • Blocking 3394 deleted

This was already moved to the next milestone in comment:5, so removing dependency.

comment:7 Changed 2 years ago by ngraham

  • Blocking set to 3469

comment:8 Changed 2 years ago by ngraham

  • Reporter changed from dconnolly to rwaitman

heron-weekly:
DC: Russ expects this in Quivira.

comment:9 Changed 2 years ago by ssuman

Chatting with Nathan, the following seem relevant:

comment:10 Changed 2 years ago by ngraham

Suman,

I think I lead you astray in comment:9. Matt and I talked with Russ and he was thinking about text-processing the ALERT_DESC to create a concept hierarchy (the direction I think you were headed before). He thought the next step could be:

  • Focus on the drug-drug interaction alerts (med_alert_type_c=1)
    • There are 1700 distinct alert descriptions for drug-drug interaction
    • Interactions are determined at the drug ingredient level.
    • In general, they look like ABCIXIMAB / ANTICOAGULANTS
  • Group the 1700 concepts in alphabetical folders (A, B, C, etc)

comment:11 Changed 2 years ago by ngraham

comment:12 Changed 2 years ago by ngraham

  • Keywords heron-weekly added

Suman,

I've narrowed the scope of this ticket based on our conversation with Russ this morning. We're trying to figure out what fits in this release - could you comment on this ticket to give your best (realistic) guess on how many coding chunks this will take (4-hour blocks) and when you can get this implemented based on coding chunks required and your foreseeable availability?

comment:13 Changed 2 years ago by ngraham

comment:14 Changed 2 years ago by ngraham

  • Keywords heron-weekly removed

heron-weekly:
SS: I'd say 2 coding chunks.
NG: Foresee this being done in this release?
SS: Yes.

comment:15 Changed 2 years ago by dconnolly

  • Cc ngraham added

Suman,

In the heron-weekly meeting when Nathan asked you for an estimate, it occurred to me that maybe we the context wasn't clear. In our development process, our heron-weekly meetings are much like XP iteration planning meetings:

While user stories are in the customer's language, tasks are in the developer's language. ... It is important for the developer who accepts a task to also be the one who estimates how long it will take to finish. People are not interchangeable and the person who is going to do the task must estimate how long it will take.
Each task should be estimated as 1, 2, or 3 (add 1/2 if you need to) ideal programming days (cf. DevTools#coding-chunks) in duration. Ideal programming days are how long it would take you to complete the task if there were no distractions. ... Tasks which are longer than 3 days should be broken down farther. ...
If the iteration has too much then the customer must choose user stories to be put off until a later iteration (snow plowing). ...
It is often alarming to see user stories being snow plowed. Don't panic. Remember the importance of unit testing and refactoring. A debt in either of these areas will slow you down. Avoid adding any functionality before it is scheduled. Just add what you need for today. Adding anything extra will slow you down. ...
Don't be tempted into changing your task and story estimates. The planning process relies on the cold reality of consistent estimates; fudging them to be a little lower creates more problems.
Keep an eye on your project velocity and snow plowing. You may need to re-estimate all the stories and re-negotiate the release plan every three to five iterations, this is normal. So long as you always implement the most valuable stories first you will always be doing as much as possible for your customers and management.

In other words,

Plans are worthless, but planning is everything.
-- Ike

comment:16 Changed 2 years ago by ssuman

  • Blocking 3469 deleted

I am able to build alphabetical folders (A, B, C, etc) and currently working on using the 128 bit key hash value for alert_desc to be used as id in concept_cd. It will be followed by iterations of unit testing, reviews and automated tests. And we want to start ETL around noon, since HERON Roadmap Planning Meeting is scheduled in the afternoon. Based on discussion with Matt, I will push the ticket for next release.

comment:17 Changed 2 years ago by ssuman

  • Milestone changed from heron-quivira-update to heron-jamestown-update

comment:18 Changed 2 years ago by ngraham

Preempted by e-mail flag feature.

comment:19 Changed 2 years ago by rwaitman

  • Blocking set to 3505

comment:20 Changed 2 years ago by dconnolly

  • Milestone changed from heron-jamestown-update to dconnolly

Milestone renamed

comment:21 Changed 2 years ago by dconnolly

  • Milestone changed from dconnolly to heron-jamestown-update

Milestone renamed

comment:22 Changed 2 years ago by ssuman

I have checked in modules for drug-drug interaction alerts (cfe87b097f30), will be beta-testing heron_etl_tiny_no_DBA build 729

comment:23 Changed 2 years ago by ssuman

  • Cc dconnolly added

comment:24 Changed 2 years ago by ngraham

  • Blocking changed from 3505 to 3505, 3536

comment:25 Changed 2 years ago by ssuman

heron_etl_tiny_no_DBA errored with - invalid SQL statement, Python lib did not recognize function

2015-06-04 16:25:06,072 ERROR heron.make_epic_alert_views.epic_alerts_transform ORA-00900: invalid SQL statement
epic_alerts_transform.sql:26: 0...
Traceback (most recent call last):
  File "structured_logging.py", line 184, in event

Replaced hashing function with hashing in sql to something as given below

select DBMS_CRYPTO.Hash(UTL_I18N.STRING_TO_RAW('sdfljadslkjzxejLEI'), 3) from dual;

heron_etl_tiny_no_DBA errored with - ORA-12899: value too large for column "HERON_ETL_14"."OBSERVATION_FACT_UPLOAD"."CONCEPT_CD" (actual: 54, maximum: 50)

2015-06-05 09:25:01,182 ERROR heron.load_epic_alerts.Epic_Best_Practice_Alerts.id.epic_facts_load ORA-12899: value too large for column "HERON_ETL_14"."OBSERVATION_FACT_UPLOAD"."CONCEPT_CD" (actual: 54, maximum: 50)
epic_facts_load.sql:28: 355...f.concept_cd,

comment:26 Changed 2 years ago by ssuman

  • Owner changed from ssuman to dconnolly
  • Status changed from new to assigned

checked in the code using branch drug_drug_3357, ran heron_etl_tiny_no_DBA, and executed a few queries, that fetched data for drug-drug interactions
heron_etl_tiny_no_DBA 736

Finished Query: "LEVOFLOXACIN / @16:17:39"
[1.4 secs]
Compute Time: 0 secs
Number of patients for "LEVOFLOXACIN / @16:17:39"
patient_count: 2

Next, I will be working on capture query and heron_test.py, and will check in.

Dan, if you could please review the code.

comment:27 Changed 2 years ago by ssuman

  • Owner changed from dconnolly to ssuman

capture_query.py automated tests are added [1cf368ab667e]. Drug-Drug Interaction Alerts added to test_heron_query.py [0fd2d40bfb18].

comment:28 Changed 2 years ago by ssuman

  • Owner changed from ssuman to dconnolly

comment:29 Changed 2 years ago by dconnolly

raw CodeReviewNotes on 0fd2d40bfb18 vs af44d987b5f6... oops; I'm late for a meeting... saving this in raw form...

source:heron_load/epic_alerts_transform.sql

/**Use Secure Hash Algorithm (SHA) for hashing alert_desc. Produces a 160-bit hash.
Key  Hex Bits
---  --------
160  db5b 26f74a31ee537c41f3b8d45027751757424  **/

I don't understand this comment. It seems to just re-state what the code does, which creates a maintenance burden for no benefit that I can see. I don't understand the example at all. Explaining why we're using this hash would be more useful.

Also: has KUH|DI: been added to our schemes table?

/* mapping of med_alert_type_c to corresponding zc table id*/
alert_type as
(select 1 as drug_drug, 50 as con_cd from dual),

why hard-code 50? why not select con_cd from zc_table where ...?

  • heron_load/epic_concepts_load.sql

spurious diffs? None of the change comments say why delete the blank line and change /*************** to /**.

--05-25-2015
added drug-drug interaction alerts (med_alert_type_c=1)

Change notes like this obscure more relevant info such as "Group the 1700 concepts in alphabetical folders".

Why nested case expressions for c_visualattributes?

Why delete the space after the comma? I'd much prefer to see a space after each comma
and around operators.

Why select distinct in al_hash_value?

Why order by c_fullname?

comment:30 Changed 2 years ago by dconnolly

  • Owner changed from dconnolly to ssuman

Suman, over to you to test one more time; I can't do it just now as Nathan has test server reserved.

Code looks good to me. I made a few tweaks, as we discussed:

  1. [274720bdd213] - collapse nested case exprs
  2. [979a09a942a7] add Drug-Drug interaction to curated schemes, along with other alert schemes
  3. [fcaa25188a4e] substr() isn't needed when building concepts either
  4. [855340cb9df3] flatten case expression
  5. [873c71b06316] When describing drug-drug concept design, leave historical notes out
  6. [d8f49a589e05] - once we have clarity_al, we don't need to join on alt_table_to_type again

comment:31 Changed 2 years ago by ssuman

Dan, Thanks, I have started the test heron_etl_tiny_no_DBA 757

ran a simple query:

Finished Query: "ACE INHIBITORS @13:50:44"
[1.4 secs]
Compute Time: 0 secs
Number of patients for "ACE INHIBITORS @13:50:44"
patient_count: 3

automated test passed

> python test_heron_query.py http://dev-app/i2b2/webclient --demo
INFO:__main__:opening Heron homepage...
INFO:__main__:Found page with title: i2b2 Web Client
INFO:__main__:Getting user configuration...
INFO:__main__:Got it.
INFO:__main__:
Testing: Drug_Drug_Interaction_Alerts
INFO:__main__:Found: 2
INFO:__main__:timing: 2015-06-12 14:32:05.486000, 2015-06-12 14:32:07.382000, 0:
00:01.896000

Merged with a2ed9fb0e564

comment:32 Changed 2 years ago by ssuman

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:33 Changed 2 years ago by ngraham

  • Keywords public-web added

comment:34 Changed 2 years ago by badagarla

  • Sensitive unset

We got permission during heron-weekly from team and documented it in ticket:3557#comment:7.

Note: See TracTickets for help on using tickets.