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

Enable PC and DPC for non-standard and source concepts #721

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

TomWhite-MedStar
Copy link

This solves issue #719, and the forum topic here.
It works in tandem with a change to WebAPI achilles_result_concept_count.
I have confirmed that this works correctly on my Databricks instance.

@TomWhite-MedStar
Copy link
Author

@chrisknoll , per discussion, removed Achilles analysis # 40

@chrisknoll chrisknoll requested a review from fdefalco August 29, 2023 12:05
@mrechkem
Copy link

@fdefalco - I wanted to follow-up to see if there have been any updates on this PR that Tom submitted.

@fdefalco
Copy link
Contributor

Unfortunately I have no updates on this PR yet. We have recently gained access to a spark environment and completed testing on PR #718. For that PR we found that the results of the queries were inconsistent with the original queries. We believe PR #756 resolves the inconsistency and improves performance on spark platforms. Can you @TomWhite-MedStar confirm?

This PR has taken a back seat to CommonDataModel publication to CRAN followed by Eunomia publication to CRAN, which was just accepted last week. Hoping to get back to testing on Achilles PR.

@mrechkem
Copy link

Thank you for the update, and I appreciate both of you working to resolve these discrepancies with the Achilles results. If I can help with the testing, please do reach out to me.

For future reference, if the development team needs access to a Databricks instance, we created one for the OHDSI community to run their packages against to ensure Databricks compatibility. If any are interested, please reach out and I’ll add you.
FYI: @pratapmaharjan

@TomWhite-MedStar
Copy link
Author

Thanks for the patch. Unfortunately, it is very slow on Spark. Using our code in production (which uses the logic from my patch), it takes 3 seconds to run analysis 717 on Databricks. Using the alternate code proposed here, the query had not generated any results after 11 minutes, so I killed the query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants