Skip to content

Iqtree3 ufboot guard - #8262

Open
d-callan wants to merge 2 commits into
galaxyproject:mainfrom
d-callan:iqtree3-ufboot-guard
Open

Iqtree3 ufboot guard#8262
d-callan wants to merge 2 commits into
galaxyproject:mainfrom
d-callan:iqtree3-ufboot-guard

Conversation

@d-callan

Copy link
Copy Markdown
Contributor

@bgruening thing i personally like least is its fasta specific

FOR CONTRIBUTOR:

  • I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • Use of AI
    • The contribution is mostly AI generated
    • The contribution has been assisted by AI
  • 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)

There are two labels that allow to ignore specific (false positive) tool linter errors:

  • skip-version-check: Use it if only a subset of the tools has been updated in a suite.
  • skip-url-check: Use it if github CI sees 403 errors, but the URLs work.

To request a review once your PR is ready, comment "please review" on the PR. This will
automatically apply the ready-for-review label if the PR is not a draft, all review
threads are resolved, and all CI checks have passed.

d-callan added 2 commits July 31, 2026 11:34
IQ-TREE requires at least 4 distinct sequences for ultrafast
bootstrap (--ufboot). By default IQ-TREE errors out when this
threshold is not met. This new optional parameter, when enabled,
silently skips --ufboot for FASTA inputs with fewer than 4 distinct
sequences. Useful when mapping over collections of alignments that
may include small ones. Default behavior (off) preserves IQ-TREE's
native error.

Also bumps version suffix to 1 and replaces dead iqtree.org URLs
with iqtree.github.io/doc.
@bernt-matthias

Copy link
Copy Markdown
Contributor

Lets wait for iqtree/iqtree3#195

The website will be back iqtree/iqtree2#538

Comment thread tools/iqtree/iqtree.xml
#if str($bootstrap_parameters.ultrafast_bootstrap.ufboot) != ''
--ufboot '$bootstrap_parameters.ultrafast_bootstrap.ufboot'
#if $bootstrap_parameters.ultrafast_bootstrap.skip_bootstrap_for_small_alignments and $general_options.s and $general_options.s.ext == 'fasta'
\$( [ "\$n_unique" -ge 4 ] 2>/dev/null && echo "--ufboot '$bootstrap_parameters.ultrafast_bootstrap.ufboot'" )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is still wrong as I said earlier. You are just executing iqtree3 ... && echo MORE_IQTREE_OPTIONS

If you really want to implement this (but IMO it should be done in iqtree itself, we should use the sequences metadata (which exists for fasta and phylip (unfortunately not for nex,clustal,msf which are just text from Galaxy's point of view).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry to chime in late. I'm infavor of this change. Iqtree is already yielding an error. From their side this seems ok. From our side this is suboptimal, as we would need to filter collections, rerun parts of a collection with different settings etc. So I thibk having an additional parameter that makes it very clear what's happening is a good way for us and more efficient

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@d-callan can you move the if out of the iqtree command? If you special conditions met in cheetah and the bash check is valid, set a env var, reuse the env var in the iqtree command later

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does the limitation also affect normal bootstrap?

I would still suggest to use sequences metadata. The sequences can be made unique using other Galaxy tools, or?

@bernt-matthias bernt-matthias left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lets wait.

@bernt-matthias

Copy link
Copy Markdown
Contributor

@d-callan and @bgruening the iqtree devs would be willing to implement the change, but noted that there will be an error for less than 3 sequences anyway (ie of there is no tree).

@d-callan

d-callan commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

thats cool i guess. im happy enough to either fix this up or close for now if theyre handling it, either way. my opinions here arent that strong, so ill wait til i hear others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage/Discuss

Development

Successfully merging this pull request may close these issues.

3 participants