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

Performance of SafeDrug and Molerec #241

Closed
flick-ai opened this issue Oct 22, 2023 · 4 comments
Closed

Performance of SafeDrug and Molerec #241

flick-ai opened this issue Oct 22, 2023 · 4 comments

Comments

@flick-ai
Copy link

Hello, esteemed author. I noticed that when running the safedrug and molerec algorithms from your library, their performance falls far short of what is claimed in your papers and benchmarks. I would like to inquire whether you used any specific parameters or techniques during testing. Thank you for your response.

@ycq091044
Copy link
Collaborator

Hello, could you please provide your training code? There are also some earlier issues related to this question which you could take a look #220 #209.

@flick-ai
Copy link
Author

Hello, I have fixed the issue based on the suggestions from the previous issue. The 'safedrug' table is now back to normal, but there are still issues with the 'molerec' table. Additionally, I noticed that you haven't implemented the 'multimarginloss' as described in the original paper for the 'safedrug' model, and the 'ddiloss' for the 'gamenet' network. Will these omissions affect the performance of these models?
Thanks.

@ycq091044
Copy link
Collaborator

It is a good question. Based on our empirical evaluation, we find the multimarginloss and ddiloss will marginally affect the model. However, the dataset processing steps in pyhealth is also slightly different from the original paper (the impact from data processing difference can be larger than adding the extra loss or not). For reproducing the exact results from the original paper, we refer you to the original paper github. We will also spent time to add the extra loss as well (which could be easy to add by your own as well).

@yangnianzu0515
Copy link
Contributor

Kindly consult our response regarding this matter.

We intend to review the specific issue at our earliest convenience.

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

No branches or pull requests

3 participants