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

Drug-Drug Interaction #18

Merged
merged 63 commits into from
Oct 4, 2022
Merged

Conversation

andrew-thach
Copy link
Contributor

@andrew-thach andrew-thach commented Sep 19, 2022

This PR is related to the issue in the main PSL repo. (Make Drug-Drug Prediction into a PSL Example #239 )

The zipped data will need to be uploaded.

EDIT 9/27/22: Data has been reuploaded for proper directory structure.

…tions. Still uses DiscreteEvaluator (need to change to AUC metrics)
…teractions splits and folds will have redundant similarity data
@andrew-thach
Copy link
Contributor Author

Hello Eriq, I believe the changes are all done. We just need to push the zipped data and maybe consider changing/disabling the weight learning.

Copy link
Member

@eriq-augustine eriq-augustine left a comment

Choose a reason for hiding this comment

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

Making some good progress.

There are several repeat comments.
Make sure to double check that you address not just the exact comment, but everything that the comment applies to.
And if you have questions, be sure to ask.

The data archive should extract into a directory named "drug-drug-interaction" (and then the respective sub directories).
This will also need to be reflected in your .data files.

drug-drug-interaction/README.md Outdated Show resolved Hide resolved
drug-drug-interaction/README.md Outdated Show resolved Hide resolved
drug-drug-interaction/README.md Show resolved Hide resolved
drug-drug-interaction/README.md Outdated Show resolved Hide resolved
drug-drug-interaction/README.md Outdated Show resolved Hide resolved
drug-drug-interaction/README.md Outdated Show resolved Hide resolved
drug-drug-interaction/README.md Show resolved Hide resolved
drug-drug-interaction/README.md Show resolved Hide resolved
drug-drug-interaction/cli/drug-drug-interaction.psl Outdated Show resolved Hide resolved
drug-drug-interaction/cli/drug-drug-interaction-eval.data Outdated Show resolved Hide resolved
Copy link
Member

@eriq-augustine eriq-augustine left a comment

Choose a reason for hiding this comment

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

Looking better.
Still a few things.

drug-drug-interaction/README.md Outdated Show resolved Hide resolved
drug-drug-interaction/README.md Outdated Show resolved Hide resolved
drug-drug-interaction/README.md Outdated Show resolved Hide resolved
drug-drug-interaction/README.md Outdated Show resolved Hide resolved
drug-drug-interaction/README.md Show resolved Hide resolved
Copy link
Member

@eriq-augustine eriq-augustine left a comment

Choose a reason for hiding this comment

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

Looks good to me!
Once we debug that weight learning issue, we can take this in.

Copy link
Member

@eriq-augustine eriq-augustine left a comment

Choose a reason for hiding this comment

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

Missed one thing.

.templates/config.json Outdated Show resolved Hide resolved
Copy link
Member

@eriq-augustine eriq-augustine left a comment

Choose a reason for hiding this comment

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

lgtm, let's just look into the WL issues.

Copy link
Member

@eriq-augustine eriq-augustine left a comment

Choose a reason for hiding this comment

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

Just needs a README fix.

drug-drug-interaction/README.md Outdated Show resolved Hide resolved
@eriq-augustine
Copy link
Member

lgtm!

@eriq-augustine eriq-augustine merged commit 4b40453 into linqs:main Oct 4, 2022
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.

2 participants