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

Add deeparg #6646

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Add deeparg #6646

wants to merge 31 commits into from

Conversation

hugolefeuvre
Copy link
Contributor

@hugolefeuvre hugolefeuvre commented Dec 19, 2024

FOR CONTRIBUTOR:

  • I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • License permits unrestricted use (educational + commercial)
  • This PR adds a new tool or tool collection
  • This PR updates an existing tool or tool collection
  • This PR does something else (explain below)

@hugolefeuvre
Copy link
Contributor Author

Problem when using deepARG in a container with the Theano package: IOError: [Errno 13] Permission denied: '/usr/local/lib/python2.7/site-packages/Theano-0.8.2-py2.7.egg-info/PKG-INFO'
This problem has already been reported and closed in this issue: nf-core/funcscan#23 but there's no clear explanation of how they solved the problem, so I have restarted the discussion to try to get more information.

@jfy133
Copy link

jfy133 commented Dec 20, 2024

Just so it doesn't get lost in the closed funcscan thread:

We ended up mounting a dummy file (the bash binary of the execution node) but with the name PKG_INFO:

https://github.com/nf-core/funcscan/blob/master/modules%2Fnf-core%2Fdeeparg%2Fpredict%2Fmain.nf#L10-L18

Also I see lower down in the script execution that we also make a directory for theanos which you may also have to do for cases of singularity (if you use that)

    # Theano needs a writable space and uses the home directory by default,
    # but the latter is not always writable, for instance when Singularity
    # is run in --no-home mode
    mkdir -p theano
    export THEANO_FLAGS="base_compiledir=\$PWD/theano"

deeparg predict <...>

I hope that helps/works for you!

@hugolefeuvre
Copy link
Contributor Author

Thank you for your answer @jfy133 ! I'll see if it works for me !

@bgruening
Copy link
Member

I did something here, but forgot what I wanted to do ;)

maybe useful hugolefeuvre#1 ?

@hugolefeuvre
Copy link
Contributor Author

I did something here, but forgot what I wanted to do ;)

maybe useful hugolefeuvre#1 ?

It was maybe about the message I send on galaxy-iuc on Element ?

Hello, is it possible to add a parameter for the docker run launched in the git tests ? Like --docker_run_extra_arguments which I can use for a planemo test --biocontainers
This would be useful for the deepARG wrapper, as the tool fails in a container and requires this parameter to be added in order to work: -v $(which bash):/usr/local/lib/python2.7/site-packages/Theano-0.8.2-py2.7.egg-info/PKG-INFO
Thank you !

And you answered

I think this is possible, but we should fix the container that is a hack at its best.

But beyond that I think it can be useful in any case.

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Thanks a lot @hugolefeuvre its now green.
I added a few more comments, they apply to both tools :)

data_managers/data_manager_deeparg/.shed.yml Outdated Show resolved Hide resolved
tools/deeparg/deeparg_predict.xml Outdated Show resolved Hide resolved
tools/deeparg/deeparg_predict.xml Show resolved Hide resolved
tools/deeparg/deeparg_predict.xml Outdated Show resolved Hide resolved
@bgruening
Copy link
Member

@bernt-matthias can you maybe look at the DM bits here?

</configfiles>
<inputs>
<param name="version" type="select" label="DB version">
<option value="1.0.4" selected="true">Data needed for running DeepARG v1.0.4</option>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use @TOOL_VERSION@ here instead of 1.0.4?

Do you know if the tool can also download other versions. Do you know if future versions can use older downloads?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think we can.
The tool download all the versions (v1 and v2), and then you can choose version with a deeparg predict parameter --model-version
There is no information if future versions can use older downloads, actually all the versions are in the zenodo that is download by deeparg as database.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you share a link to the zenodo record?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

data_managers/data_manager_deeparg/data_manager_conf.xml Outdated Show resolved Hide resolved
data_managers/data_manager_deeparg/data_manager_conf.xml Outdated Show resolved Hide resolved
<param name="input" type="data" format="fasta" label="Input file"/>
<param name="deeparg_db" type="select" label="DeepARG database">
<options from_data_table="deeparg_database_versioned">
<validator message="No deeparg database is available" type="no_options"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we filter for the version here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What type of filter did you have in mind?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking to filter for the version, such that the corresponding tool version can only process the values generated with exactly this version .. not sure if it makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make sure that the only DB that can be used is the one that corresponds to the version of the tool?
Someting like that ? <filter column="version" value="@TOOL_VERSION@"/>

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.

5 participants