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

Xchem deep #951

Merged
merged 21 commits into from Mar 27, 2020
Merged

Xchem deep #951

merged 21 commits into from Mar 27, 2020

Conversation

tdudgeon
Copy link
Contributor

Initial version currently bypassing GPU execution by use of the --mock flag

@simonbray
Copy link
Collaborator

@bgruening, what is actually the goal of this PR? Do we want to deploy the tool on usegalaxy.eu? If so we should probably remove the --mock tag and do some more testing.

@bgruening
Copy link
Owner

Yes the goal is to deploy it on usegalaxy.eu. But before we need to test it with docker on some GPU enabled host without the --mock option.

@simonbray
Copy link
Collaborator

But we should do that before merging here, right?

<!--requirement type="package" version="3.0.0">openbabel</requirement-->
<!--requirement type="package" version="3.7">python</requirement-->
<!-- many other requirements are needed -->
<container type="docker">informaticsmatters/deep-app-ubuntu-1604:latest</container>
Copy link
Owner

Choose a reason for hiding this comment

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

this should be a tagged version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't have one yet. But we will at some stage soon.

An SDF file is produced as output. The binding affinity scores are contained within the SDF file
as the XChemDeepScore property. Values range from 0 to 1 with 0 being bad and 1 being good.

]]></help>
Copy link
Owner

Choose a reason for hiding this comment

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

no citation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think the paper has been submitted yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can still write a citation in bibtex (e.g. linking to a github repo) - we also do this for rdkit because there is no publication

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've asked if there is anything we can use.

@bgruening
Copy link
Owner

But we should do that before merging here, right?

I had hoped @tdudgeon has done this. But yes we should do the testes before :)

@bgruening
Copy link
Owner

@tdudgeon you mentioned somewhere that Docker needs to be specially enabled for GPU, can you add some infos about this?

@simonbray
Copy link
Collaborator

Can you reduce the size of the test data?

@tdudgeon
Copy link
Contributor Author

Can you reduce the size of the test data?

Done tdudgeon@8b955eb

@tdudgeon
Copy link
Contributor Author

@tdudgeon you mentioned somewhere that Docker needs to be specially enabled for GPU, can you add some infos about this?

The docker daemon needs to be a special one that supports GPUs.
I think this describes the details: https://github.com/NVIDIA/nvidia-docker
@alanbchristie knows the details. He can advise on this next week.

@bgruening
Copy link
Owner

@tdudgeon @alanbchristie have you ever tried singulairy for GPUs?

This PR is fine for me it needs some testing on a real GPU enabled host.

@tdudgeon
Copy link
Contributor Author

have you ever tried singulairy for GPUs?
Not yet. We plan to do so.

@reskyner
Copy link
Contributor

Hi Björn, can we get this merged and tested on the real infrastructure? I think there's a lot of difficulty for us getting access to a GPU to test here.

@reskyner
Copy link
Contributor

Hi @bgruening @simonbray,

Have we gotten any further on this? We're currently working on a fragment screen for coronavirus. It would be awesome if we could get this tool deployed to work in sync with that screen. (to suggest new compounds)

Let me know if there's anything else we need to do

Thanks,
Rachael

@bgruening
Copy link
Owner

bgruening commented Feb 25, 2020

I was trying to run it locally, but get this ...

|  Traceback (most recent call last):
|  File "/home/bag/.planemo/planemo_tmp_rWhH6c/xchem_deep_score.py", line 204, in <module>
|  main()
|  File "/home/bag/.planemo/planemo_tmp_rWhH6c/xchem_deep_score.py", line 200, in main
|  execute(args.input, args.receptor, args.outfile, mock=args.mock)
|  File "/home/bag/.planemo/planemo_tmp_rWhH6c/xchem_deep_score.py", line 180, in execute
|  scores = read_predictions()
|  File "/home/bag/.planemo/planemo_tmp_rWhH6c/xchem_deep_score.py", line 125, in read_predictions
|  with open("{0}{1}{2}".format(work_dir, os.path.sep, predict_file_name), 'r') as input:
|  FileNotFoundError: [Errno 2] No such file or directory: './predictions.txt'

open("{0}{1}{2}".format(work_dir, os.path.sep, predict_file_name), 'r')

This is also wired? os.path.join(work_dir, predict_file_name) should work, but why r if this is an output file?

@tdudgeon
Copy link
Contributor Author

@bgruening It runs locally for me:
planemo t --docker xchem_deep_score.xml

Possibly the docker image was not up to date (though I doubt it). I just pushed the version I used, and also tagged it as 1.0.

The predictions.txt is opened in read mode as by that stage it has been created and it's being read to extract the scores.

@simonbray
Copy link
Collaborator

simonbray commented Mar 5, 2020

(Please ignore this comment.)

@tdudgeon
Copy link
Contributor Author

tdudgeon commented Mar 5, 2020

Damn it! My hacking about lost the mock part and symlinking the ligands.
This has now been fixed in 8641398
Remember to add the --docker arg when you run planemo!

@tdudgeon
Copy link
Contributor Author

tdudgeon commented Mar 5, 2020

I don't know if it's possible to tell planemo that it needs to run with --docker and with --gpus all.
Without both we can't run proper tests with and without a GPU.

@bgruening bgruening merged commit 854a3bb into bgruening:master Mar 27, 2020
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.

None yet

4 participants