drug_cost table (+5.0.1 version issues)

[vojtechhuser]

with CDM 5.0.1 unified cost table, some of the Achilles analyses break on 5.0.1 CDM

[cgreich]

What are you suggesting? Spawn a new Achilles version?

[chrisknoll]

5.0.1 shouldn't add any breaking changes, if it introduces a new table, fine, the existing tables should continue to be there. Any breaking changes should be introduced in 6.0. (or 5.1 depending how they version things).

To account for the new tables in 5.0.1, it might make sense to add a new set of analyses for those table into Achilles, but it shoudl degrade gracefully if the table isn't thee (or we do a version check and not execute those analyses if those tables are gone).

[vojtechhuser]

On Lee's test server, it does not degrade gracefully. That CDM has no drug_cost table. SQL code does not see it and ...

It fails with an error. The solution is to use the analysis parameter and make achilles skip cost analyses .

[jmbanda]

I also see the saw the same issue as Vojtech when I downloaded the CDM tables after the change. They now have the original cost tables commented, thus they don't get created. This makes Achilles break, I had to just add the old tables to solve the issue.

---

Juan M. Banda, Ph.D. Research Scientist - Center for Biomedical Informatics Research Stanford University School of Medicine

Google Scholar: http://goo.gl/RiO2jY | ResearchGate: http://goo.gl/7C31uc DBLP: http://goo.gl/BBvmJj | LinkedIn: http://goo.gl/h95tnq

On Wed, May 25, 2016 at 10:04 AM, Vojtech Huser notifications@github.com wrote:

On Lee's test server, it does not degrade gracefully. That CDM has no drug_cost table. SQL code does not see it and ...

It fails with an error. The solution is to use the analysis parameter and make achilles skip cost analyses .

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/OHDSI/Achilles/issues/110#issuecomment-221638112

[chrisknoll]

Ok, looks like 5.01 does introduce breaking changes. Looks like a new release of Achilles is required.

I know internally we're not using the the new cost table structure, So for the short term, I feel we should be supporting the old 5.0 schema, but It's a bit unknown how to handle this in the achilles script (since it is one big script that executes). Maybe something with sql render and 'if then' blocks can make this work.

@cgreich : For the future, minor version releases of software/specs should not introduce massive schema changes (and dropping a table qualifies as 'massive'). Instead, for the interim, both tables should be available, and a clear deprecation path should be laid out for those tables. I think for 5.1 it should have introduced those new tables (leaving existing alone) and 6.0 removes the deprecated tables. Just my 2 cents. What Achilles would do to support both tables of data would be to UNION the existing domain-specific cost tables with the generic cost table with a WHERE clause of 'where domain={domain type}. That way we have seamless transition when people move from domain-specific to generic cost tables in their CDM.

-Chris

[chrisknoll]

@cgreich : I jumped over to the git repo and noticed that the DDL that is there seems to have the new cost table added, but only a comment about retiring the old cost tables. So seems like if that's the case, then they are introducing the new tables, and phasing out the old ones. If that's so, that's good! Let's leave both tables in the schema and leave it to the ETL implementers to decide which tables to use (but with the understanding that the legacy cost tables are on the way out).

For Achilles, we can modify the cost queries to UNION across both tables to get the data. that way it'll work in both contexts.

@leeevans , can you restore the old cost tables to the test server while we work out these issues? We don't want to have the tool builds break.

[chrisknoll]

@cgreich : sorry, I looked closer in the commits: the tables are commeted out :( That doesn't jive with the 'willb e phased out' comment in the code :)

Can we plase leave both tables in there for now.

[leeevans]

@vojtech The SQL Server cloud instance you are using for Achilles/IRIS dev/testing was built with the latest (5.0.1) CDM github code. If you would like to create the original 5.0 cost tables in that database you can un-comment and execute their DDL from the below SQL: https://github.com/OHDSI/CommonDataModel/blob/master/Sql%20Server/OMOP%20CDM%20ddl%20-%20SQL%20Server.sql

@chrisknoll The CI build server uses a different postgres OHDSI cloud database which is basically still running 5.0 (with one 5.0.1 change to the drug_strength table so the vocab data load works for that table). So the original 5.0 cost tables still exist in the CI build server database and the build should be fine.

[vojtechhuser]

if we can fix the travisCI so that the current master branch passes (and it should pass, it has no errors), that would be great asset to my development of new measures (facilitate future pull requests) @leeevans

[cgreich]

@chrisknoll et al.:

I know it's a pain. With respect to the COST table we decided to do it the way we did (rather 'massive') because the old one did not work, plus hardly anybody used it (becasue it didn't work and becuase few people did cost). So, that is why we decided to make the rather big swap. It was the first order of action when the new CDM group was formed. It will not happen again. Let's not continue the 5.0 COST table structure.

[chrisknoll]

@cgreich, the community using Achilles weeps at you calling them 'hardly anyone' :)

I was at those meetings and I understand the new structure was thoroughly discussed, but I must have missed the part about removing the old tables and discussing a migration plan for the tools that depend on it. So, apologies if I missed this and wasn't informed

Since this is a new release with schema changes, do you think a formal release could be announced on the ohdsi forums (I think I searched for it but coudln't find it), and also the releases found at the git repo: https://github.com/OHDSI/CommonDataModel/releases does not show a 5.0.1 release (with a set of changes included). That would let people know what to expect when they install the new cdm schema.

For this issue, I can modify Achilles to make running the cost analysis an optional parameter (I'll default it to FALSE) but if someone knows that the're using the old cost tables and they want those statistics, they can pass TRUE in that parameter. That will let this code run (by default) on the new schema but without COST statistics (which someone will need to implement the new queries to generate those statistics on the new structure).

[cgreich]

@chrisknoll:

The community using Achilles, or better, the community maintaining Achilles and trying to make it work all over the place weeps, of course. The community use the old COST table is congruent to "hardly anybody", and therefore hardly weeps. :)

I need to ping you next week and we should do it together. Otherwise, I will screw it up. Can we?

[chrisknoll]

Sure no problem! I'm prepping this update to Achilles now, I can set up a webex to walk through it if you like. Let me know what days/times are good for you and I'll set it up.

[cgreich]

@chrisknoll: Cool. Will do.

[chrisknoll]

I pushed in a change to address this: we now default to skipping the cost analysis (but you can pass in a flag runCostAnalysis into achilles() and it will execute that part of the script).

Note this depends on a fix in SqlRender, so you must uninstall-reinstall the SqlRender R package before kicking this off again. Also note I had issues uninstalling SqlRender, I had to restart the enviornment, delete package, then reinstall for it to work, but it does work.