Add structureclusterupdate workflow - #588
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new structureclusterupdate workflow command to extend an existing structural clustering with new sequences, driven by a bundled shell pipeline.
Changes:
- Introduces
structureclusterupdateC++ entrypoint that prepares parameters, temp directory handling, and executes the embedded workflow script. - Registers the new command in the CLI command list and build system.
- Adds and compiles a new
data/structureclusterupdate.shworkflow script resource.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/workflow/StructureClusterUpdate.cpp | Implements the new workflow command wrapper and script execution. |
| src/workflow/CMakeLists.txt | Includes the new workflow source in the build. |
| src/LocalCommandDeclarations.h | Declares the new command entrypoint for linkage. |
| src/FoldseekBase.cpp | Registers the new structureclusterupdate command in the CLI. |
| data/structureclusterupdate.sh | Adds the pipeline logic for updating clustering with new sequences. |
| data/CMakeLists.txt | Compiles the new script into the binary resources. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void setStructureClusterUpdateDefaults(LocalParameters *p) { | ||
| p->covThr = 0.8; | ||
| p->evalThr = 0.001; | ||
| p->sortByStructureBits = 0; | ||
| p->alignmentMode = Parameters::ALIGNMENT_MODE_SCORE_COV_SEQID; | ||
| p->compBiasCorrection = 0; | ||
| } | ||
|
|
||
| void setStructureClusterUpdateMustPassAlong(LocalParameters *p) { | ||
| p->PARAM_ALIGNMENT_MODE.wasSet = true; | ||
| } |
| cmd.addVariable("REMOVE_TMP", par.removeTmpFiles ? "TRUE" : NULL); | ||
| cmd.addVariable("RECOVER_DELETED", par.recoverDeleted ? "TRUE" : NULL); |
| cmd.execProgram(program.c_str(), par.filenames); | ||
|
|
||
| // Unreachable | ||
| assert(false); | ||
| return EXIT_FAILURE; |
| if notExists "$NEWCLUST"; then | ||
| log "=== Merge updated clustering with new clusters" | ||
| # shellcheck disable=SC2086 | ||
| "$MMSEQS" concatdbs "${UPDATEDCLUST}" "${TMP_PATH}/newClusters" "$NEWCLUST" --preserve-keys ${THREADS_PAR} \ | ||
| || fail "concatdbs died" | ||
| fi |
| } | ||
|
|
||
| # check number of input variables | ||
| [ "$#" -ne 6 ] && echo "Please provide <i:oldSequenceDB> <i:newSequenceDB> <i:oldClusteringDB> <o:newMappedSequenceDB> <o:newClusteringDB> <o:tmpDir>" && exit 1 |
|
Hi Antonio, thank you very much for adding this. I looked a bit through the code and ran it a couple of times. I have a couple of comments: 1- 2- I didn't see in the shell parts that also updates the 3- If we expose the recover deleted branch, I see that in that part in the shell workflow, only the main DB and the 4- The variable 5- From my understanding, structural prefiltering is usually done on the 3Di sequences and this command does create 3Di subsets I don't think there's anything major that needs to change, I will try to push my regression test before the end of the week, I am a bit sick today. |
- expose --recover-deleted via own param list (structureclusterupdate) so it shows in help without leaking onto structurecluster - add _ca everywhere _ss is handled (renamedbkeys/createsubdb for newMappedDB, NEWDB.newSeqs, OLDDB.repSeq; recover branch too) - recover branch: rebuild _ss and _ca for NEWDB.withOld, not just main+_h - split monolithic notExists guard in recover branch into per-sidecar guards so partial-failure retry rebuilds missing sidecars - fix hash to use par.structureclusterupdate (includes --recover-deleted) so tmp dir is not reused across runs that differ only in that flag - prefilter on _ss subsets to match structurecluster convention - drop unused RESULT2REPSEQ_PAR - guard OLCLUST cleanup in non-recover path against missing files - add _ss/_ca to help example in FoldseekBase.cpp - fix notExists check to use .dbtype suffix
|
thanks @fawaz-dabbaghieh, all five were on point. pushed an update:
also cleaned up the new temp dbs and took the bot's |
5ad73aa to
4ff4966
Compare
Mirror StructureSearch.cpp GPU block into StructureClusterUpdate.cpp. Add ungappedprefilter to structureclusterupdate param list. Branch .sh prefilter call on PREFMODE; pad OLDDB.repSeq for GPU path. --gpu is opt-in; CPU k-mer prefilter remains the default.
Align new sequences against the padded representative DB and remap the swapped hit keys back to original rep keys before merging with the old clustering. The remap read the lookup name column (col2) instead of the original key (col3), so renamedbkeys got unparseable targets and collapsed every join-target key to 0 -- all structurally-assigned new sequences then merged into the single cluster keyed 0 instead of their best-hit cluster. Use col3 (originalKey) from the mmseqs lookup format.
This PR adds
structureclusterupdate, an incremental update workflow forstructural cluster databases. Given an existing cluster database and a new
(larger) sequence database containing additional structures, it efficiently
updates the cluster assignments without re-running a full clustering from
scratch.
Approach
Uses
diffseqdbsto partition sequences into three groups by integer key:representatives are remapped to new key space via
renamedbkeysrepresentatives with structural alignment (
structurealign), then assignedor seeded into new clusters
(or re-appended with new keys when
RECOVER_DELETED=1)All prefiltering uses the 3Di alphabet (
--seed-sub-mat aa:3di.out) andalignment uses
structurealign, so the update is fully structure-basedthroughout.