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 SKIP_ORF option in translated_search #885

Merged
merged 11 commits into from
Nov 22, 2024

Conversation

LunaJang
Copy link
Contributor

@LunaJang LunaJang commented Sep 2, 2024

added a new parameter, and changed translated_search.sh to run extractframes and translatenucs instead of extractorf according to the user's choice

data/workflow/translated_search.sh Outdated Show resolved Hide resolved
if [ "$ORF_SKIP" ]; then
if notExists "${TMP_PATH}/t_orfs_aa.dbtype"; then
# shellcheck disable=SC2086
"$MMSEQS" extractframes "$1" "${TMP_PATH}/t_orfs_nucl" ${EXTRACT_FRAMES_PAR} \
Copy link
Member

Choose a reason for hiding this comment

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

This should be "$2".

TARGET="${TMP_PATH}/t_orfs_aa"
TARGET_ORF="${TMP_PATH}/t_orfs_aa"
TARGET="${TMP_PATH}/t_orfs_aa"
TARGET_ORF="${TMP_PATH}/t_orfs_aa"
Copy link
Member

Choose a reason for hiding this comment

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

Please check if this whitespace should be here. From the diff, this looks wrong.

@@ -172,6 +172,7 @@ Parameters::Parameters():
PARAM_ORF_FILTER_S(PARAM_ORF_FILTER_S_ID, "--orf-filter-s", "ORF filter sensitivity", "Sensitivity used for query ORF prefiltering", typeid(float), (void *) &orfFilterSens, "^[0-9]*(\\.[0-9]+)?$"),
PARAM_ORF_FILTER_E(PARAM_ORF_FILTER_E_ID, "--orf-filter-e", "ORF filter e-value", "E-value threshold used for query ORF prefiltering", typeid(double), (void *) &orfFilterEval, "^([-+]?[0-9]*\\.?[0-9]+([eE][-+]?[0-9]+)?)|[0-9]*(\\.[0-9]+)?$"),
PARAM_LCA_SEARCH(PARAM_LCA_SEARCH_ID, "--lca-search", "LCA search mode", "Efficient search for LCA candidates", typeid(bool), (void *) &lcaSearch, "", MMseqsParameter::COMMAND_PROFILE | MMseqsParameter::COMMAND_EXPERT),
PARAM_ORF_SKIP(PARAM_ORF_SKIP_ID, "--orf-skip", "ORF skip mode", "???", typeid(bool), (void *) &orfSkip, ""),
Copy link
Member

Choose a reason for hiding this comment

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

Still TODO

Copy link
Member

Choose a reason for hiding this comment

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

PARAM_ORF_SKIP(PARAM_ORF_SKIP_ID, "--orf-skip", "Extract frames instead of ORFs", "Extract frames instead of ORFs during translated search", typeid(bool), (void *) &orfSkip, ""),

Copy link
Member

Choose a reason for hiding this comment

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

@martin-steinegger any better ideas for a parameter name?

cmd.addVariable("ORF_SKIP", "TRUE");
cmd.addVariable("TRANSLATE_PAR", par.createParameterString(par.translatenucs).c_str());
cmd.addVariable("EXTRACT_FRAMES_PAR", par.createParameterString(par.extractframes).c_str());
}
Copy link
Member

Choose a reason for hiding this comment

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

code/whitespace style: please do } else {

# shellcheck disable=SC2086
"$MMSEQS" translatenucs "${TMP_PATH}/t_orfs_nucl" "${TMP_PATH}/t_orfs_aa" ${TRANSLATE_PAR} \
|| fail "translatenucs died"
"$MMSEQS" rmdb "${TMP_PATH}/t_orfs_nucl"
Copy link
Member

Choose a reason for hiding this comment

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

The rmdb calls need ${VERBOSITY}:

# shellcheck disable=SC2086
"$MMSEQS" rmdb "${TMP_PATH}/t_orfs_nucl" ${VERBOSITY}

# shellcheck disable=SC2086
"$MMSEQS" translatenucs "${TMP_PATH}/q_orfs_nucl" "${TMP_PATH}/q_orfs_aa" ${TRANSLATE_PAR} \
|| fail "translatenucs died"
"$MMSEQS" rmdb "${TMP_PATH}/q_orfs_nucl"
Copy link
Member

Choose a reason for hiding this comment

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

see below

data/workflow/translated_search.sh Outdated Show resolved Hide resolved
# shellcheck disable=SC2086
"$MMSEQS" extractorfs "$2" "${TMP_PATH}/t_orfs_aa" ${ORF_PAR} \
|| fail "extract target orfs step died"
if [ "$ORF_SKIP" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

same as above

src/workflow/Search.cpp Show resolved Hide resolved
@LunaJang
Copy link
Contributor Author

I agree the PR can be merged under the MIT license

@milot-mirdita milot-mirdita merged commit 0310e6b into soedinglab:master Nov 22, 2024
15 checks passed
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