Update deprecated Dask `LocalCluster` method parameters in PyRosetta unit tests (#726)
This quick PR aims to update some PyRosetta tests' usage of the
deprecated `diagnostics_port` parameter in Dask's `LocalCluster`
constructor, resulting in the following error since release `2026.6.0`:
```
TypeError: Server.__init__() got an unexpected keyword argument 'diagnostics_port'
```
Modernize pre-C++11 copy prevention to = delete across core/ and protocols/ (#700)
## Summary
Replace the pre-C++11 idiom (declared-but-undefined private copy ctor /
copy assignment used to prevent copies via link error) with explicit `=
delete`. 16 files, no behaviour change — just turning latent link errors
into clear compile-time errors. Same kind of work as #685 / #674 / #672
/ #671 / etc.
Two flavours:
1. **Stand-alone non-copyable classes** (14 files): replace the
unimplemented private declarations with `= delete`. Drops redundant
comments (`"deny use of the copy constructor"`, `"private and
unimplemented"`) that `= delete` already conveys. Keeps the explanatory
rationale only where the *why* is non-obvious (`ScoreFunction` /
`DockingScoreFunction` / `OtherContextScoreFunction`: copies would
discard subclass information; use `clone()` instead).
2. **Singleton** (`SymDofMoverSampler`, inheriting
`utility::SingletonBase`): drop the redundant copy/assignment
declarations entirely — the base already `= delete`s its own copies,
which transitively deletes the derived copies.
### Files touched
`core/scoring/`: `ScoreFunction.hh`, `MinScoreScoreFunction.hh`,
`DockingScoreFunction.hh`
`core/pack/rotamer_set/`: `RotamerSet.hh`, `RotamerSet_.hh`,
`RotamerSubset.hh`
`core/conformation/`: `RotamerSetBase.hh`
`core/environment/`: `DofPassport.hh`
`core/fragment/`: `ConstantLengthFragSetIterator_.hh`,
`FrameListIterator_.hh`, `MinimalFragSetIterator_.hh`,
`OrderedFragSetIterator_.hh`
`basic/resource_manager/locator/`: `FileSystemResourceLocator.hh` (the
inner `FileStream`)
`protocols/flexpack/`: `OtherContextScoreFunction.hh`
`protocols/frag_picker/`: `FragmentPicker.hh` (the inner `QuotaDebug`)
`protocols/matdes/`: `SymDofMoverSampler.hh`
Apply Rule of Zero to remaining LREnergyContainer iterator subclasses (#699)
## Summary
Extends the disulfide-iterator pattern from PR #689 to the four
remaining `ResidueNeighbor{,Const}Iterator` subclasses in
`core/scoring/`:
- `DenseNeighbor{,Const}Iterator` (`DenseEnergyContainer.{hh,cc}`)
- `OneToAllNeighbor{,Const}Iterator` (`OneToAllEnergyContainer.{hh,cc}`)
- `PolymerBondedNeighbor{,Const}Iterator`
(`PolymerBondedEnergyContainer.{hh,cc}`)
- `CstResNeighb{,Const}Iterator`
(`constraints/ConstraintEnergyContainer.{hh,cc}`)
For each class:
- Remove the empty out-of-line destructor (`~X() override;` decl in
`.hh` plus `X::~X() = default;` in `.cc`) — Rule of Zero suffices since
the base destructor is already virtual.
- Replace the pre-C++11 private undefined derived-derived
copy-assignment (`X & operator = (X const & );`) with an explicit `=
delete` and a short comment explaining that all assignment must funnel
through the polymorphic `operator = ( ResidueNeighbor{,Const}Iterator
const & )` so derived members are downcast and copied correctly.
Together with PR #689 this exhausts the `LREnergyContainer` iterator
family — every concrete container's iterator pair now follows the same
idiom.
Apply Rule of Zero to old-style uncopyable classes in core/ and protocols/ (#697)
## Summary
Modernize 31 headers across `core/` and `protocols/` that used the
pre-C++11 idiom of declaring (but not defining) private copy constructor
and assignment operator to forbid copies. The link-error-as-enforcement
trick is fragile (silent breakage if anyone defines them in the same
translation unit, confusing diagnostics on misuse, and falsely implies
the class has a copy constructor).
Two flavours of fix applied:
1. **Stand-alone non-copyable classes** — InteractionGraphBase
node/edge/graph family (9 files, 23 classes total),
FlexbbInteractionGraph (2 classes), JobDigraph (2 classes),
AtomTreeCollection's ResidueAtomTreeCollection, Matcher, and
MonteCarlo's `operator=`: replace the private unimplemented declarations
with public `= delete`. Where the old assignment took a non-const
reference or returned `T const &` (a C++03 idiom holdover), the
modernized signatures take `T const &` and return `T &`.
2. **Singleton-derived factories** (`utility::SingletonBase` children) —
`RotamerLibrarySpecificationFactory`,
`SingleResidueRotamerLibraryFactory`, `RotamerLibrary`, `CenrotLibrary`,
plus `DockingHighResFactory`, `EvaluatorFactory`, `JobInputterFactory`,
`JobOutputterFactory`, `LoopMoverFactory`,
`LoopRefineInnerCycleFactory`, `LoopsDefinerFactory`, `GridFactory`,
`PoseSelectorFactory`, `RotamerRecoveryFactory`,
`AssemblyRequirementFactory`, `AssemblyScorerFactory`: simply drop the
redundant unimplemented copy/assignment declarations. `SingletonBase`
already `= delete`s its own copy/assignment, which transitively deletes
the derived class's implicit versions, so the extra lines were noise.
No behaviour change: copy/assignment attempts that were link errors
before are now compile errors, which is the intended diagnostic-quality
improvement. Debug build passes clean.
Support chain designations of multiple letters. (#551)
Part of the "better support for mmCIF format" effort.
The mmCIF format supports larger systems with multi-character chain
designations. This PR changes the internals of Rosetta to support
multi-letter chain designations. There shouldn't be any significant
behavioral changes if you just continue to use single letter chain
designations.
The most significant interface change was the change in interface
designation. Instead of strings like "A_B", code now generally takes a
`core::pose::DockingPartners` object. (This can be created from the
string designation, to ease IO - it should be invisible except perhaps
at the PyRosetta level.)
Places that needed to be fixed were discovered primarily with type
change issues during compilation. I may have missed some places which
need updated support.
Fix incorrect comparison logic in operator< / difference_from (#696)
## Summary
Four independent classes had broken comparison logic that violated
either the contract of \`operator<\` (strict weak ordering) or the
intended semantics of \`difference_from\`. Each is a separate, surgical
fix bundled into a single PR because they all share the same
\"comparison-operator misuse\" theme.
* **\`core/chemical/sdf/mol_util.cc\`** — \`BondData::operator<\` was
\`(lower < other.lower) || (upper < other.upper)\`. This is not a strict
weak ordering: for example, \`(5, 1)\` and \`(3, 7)\` each compare less
than the other, breaking antisymmetry. \`BondData\` is held in
\`std::set<BondData>\` (see \`parse_bond_type_data\`), so the broken
ordering directly affects the set. Replaced with a proper lexicographic
compare via \`std::tie\`.
* **\`core/scoring/motif/motif_hash_stuff.cc\`** —
\`ResPairMotif::operator<\` returned \`0 < memcmp(...)\`, which is true
exactly when \`memcmp\` says *this > other*; the comparison was
inverted. Switched to \`memcmp(...) < 0\` (matches
\`MotifHit::operator<\` in the same file).
* **\`core/io/StructFileReaderOptions.cc\`** — \`operator<\` used \`==\`
instead of \`!=\` for the short-circuit \`return false\` lines. The
intended pattern (used correctly by the parent
\`StructFileRepOptions::operator<\` and by
\`ImportPoseOptions::operator<\`) is \`if a < b return true; if a != b
return false; // fall through\`. The \`==\` form returns false when
members are equal, short-circuiting before the rest of the members are
compared, *and* falls through when one member is greater. Fixed every
\`==\` to \`!=\` and added the missing \`!=\` follow-up after
\`glycam_pdb_format_\`.
* **\`core/chemical/gasteiger/GasteigerAtomTypeData.cc\`** —
\`difference_from\` short-circuited \`return Other\` when \`charge_ ==
OTHER.charge_ || element_type_ != ... || ...\`. The first clause is
inverted: the function should return \`Other\` when fundamental
properties *differ*, not when they match. The bug also made the
lone-pair / s-orbital / p-orbital classification logic below this check
unreachable for any two types with equal charge. Fixed \`==\` to \`!=\`.
Fix loading of ligands when three letter code matches NCAA (#480)
When a ligand params file provided with
`-extra_res_fa` has a three letter code which matches an NCAA three
letter code from the database, that ligand ResidueType is never selected
on PDB read-in, even if it's a much better match for the names in the
PDB.
The reason for this is that the PDB reader residue typer prioritizes
patched polymeric terminus types (those with TERMINUS properties) for
residues at the ends of chains, discarding the ligand types as a
possibility before even encountering the name-based selection.
This PR adjusts how the typer selects residues. Instead of having
chain-terminal residues preferring terminus properties, actually look at
the connection points, and look for residues which have/don't have the
UPPER & LOWER connection points. (This is really what
"is_lower_terminus" and "is_upper_terminus" in PoseFromSFRBuilder
signifies: is this residue polymerically connected to the adjacent
residue.)
Fix off-by-one bugs in 1-indexed loops over pose residues (#695)
## Summary
Replace \`i != end\` with \`i <= end\` (or \`i != size()\` with \`i <=
size()\`) in several loops that iterate over 1-indexed residue ranges.
With the old condition, the last residue (index == end) was silently
skipped.
* \`core/pack/task/PackerTask_.cc\`: pymol-style debug selection output
omitted the last residue.
* \`protocols/forge/components/BDR.cc\`:
- the full-atom check used to gate \`switch_to_residue_type_set\`
skipped the last residue, so a non-full-atom last residue could bypass
conversion and later cause sidechain-restore mismatch (the precise
failure the surrounding comment warns about).
- the loop building the \`RestrictResidueToRepacking\` operation skipped
the last residue, so a C-terminal residue outside \`new_positions\` was
designed instead of repack-only.
* \`protocols/fldsgn/BluePrintBDR.cc\`: same full-atom check bug as in
\`BDR.cc\`.
* \`protocols/denovo_design/components/StructureData.cc\`:
\`compute_cutpoints\` skipped the last residue, dropping a
\`CUTPOINT_LOWER\` variant on the C-terminal residue.
* \`protocols/enzdes/BackboneSampler.cc\`: the user-requested number of
backbone-Monte-Carlo trials was off by one (e.g. \`bb_moves_=1000\`
actually ran 999 trials, in contradiction with the \"Running N
trials...\" log line printed just above).
These are all instances of the same pattern: a 1-indexed loop using
\`!=\` against a one-past-the-last bound that was actually meant to be
the last valid index. Each loop body uses the index directly to access
pose data, so the fix is to switch the comparison to \`<=\`.
Apply Rule of Zero across remaining utility/ empty destructors (#694)
## Summary
Bundle of small Rule-of-Zero / clarity fixes across `utility/` for
classes
not covered by any other open PR. All changes are observably no-ops at
runtime (each destructor body either was empty or `= default`); the goal
is to remove redundant declarations and document a deliberate
non-trivial
case.
- **Remove empty `~Foo() {}`** where the implicit destructor is already
correct (virtual is preserved via the base class when relevant):
`Bound`, `Exception`, `ocstream`, `AutoKey`, `UserKey`.
- **Convert `~Foo() {}` to `~Foo() = default;`** for polymorphic root
classes (no virtual base destructor to inherit), so the destructor
stays virtual: `Show`, `WidgetFactory`, `irstream`, `orstream`, `Key`,
`Option`.
- **Drop matching `= default` destructor pairs (.hh decl + .cc def)**
for
classes that inherit from `utility::VirtualBase`, which already
supplies a `virtual ~VirtualBase() = default;`: `heap`,
`subset_mapping`, `recent_history_queue`, `GeneralFileContents`,
`GeneralFileContentsVector`, `Tag`.
- **`utility/io/mpistream.hh`**: the destructor of `basic_mpi_streambuf`
is **not** trivial — it calls `flush_final()`, which sends the close
message on the MPI channel. Add explicit `= delete` for its copy
constructor and copy-assignment so an accidental copy can't trigger
the side effect twice. Also replace the empty
`~basic_mpi_ostream() override {}` with `= default`.
Apply Rule of Zero across utility/options/ empty destructors (#693)
## Summary
Drop user-declared empty destructors from the `utility/options/` and
`utility/options/keys/` class hierarchies. The implicit destructor
preserves virtual dispatch in every case via inherited virtual
destructors:
- All option classes inherit (transitively) from
`utility::options::Option`, which declares `virtual ~Option()`.
- All option-key classes inherit (transitively) from
`utility::keys::Key`, which declares `virtual ~Key()`.
No class touched here owns a raw resource, declares a non-trivial
destructor body, or has a user-declared copy/move that would suppress
the implicit destructor.
### `utility/options/` — abstract bases and remaining leaf option
classes
* Abstract / templated bases: `ScalarOption`, `ScalarOption_T_`,
`VectorOption`, `VectorOption_T_`, `AnyOption`, `AnyVectorOption`.
* Leaf options: `PathOption`, `PathVectorOption`, `StringOption`,
`StringVectorOption`, `ResidueChainVectorOption`.
`AnyOption` / `AnyVectorOption` used the `virtual` keyword on the empty
body; the rest used `override {}`. Both forms are equivalent to the
implicit virtual destructor here.
### `utility/options/keys/` — base + 16 leaf key types
* Base: `OptionKey`.
* Leaves: `AnyOptionKey`, `AnyVectorOptionKey`, `BooleanOptionKey`,
`BooleanVectorOptionKey`, `FileOptionKey`, `FileVectorOptionKey`,
`IntegerOptionKey`, `IntegerVectorOptionKey`, `PathOptionKey`,
`PathVectorOptionKey`, `RealOptionKey`, `RealVectorOptionKey`,
`ResidueChainVectorOptionKey`, `ScalarOptionKey`, `StringOptionKey`,
`StringVectorOptionKey`, `VectorOptionKey`.
This is the natural follow-up to #692, which intentionally deferred this
batch. With this PR, the entire `utility/options*`
empty-virtual-destructor pattern is cleaned up.
29 files, 156 deletions, 0 additions. Debug build clean.
Apply Rule of Zero across utility/ helpers (#692)
## Summary
Remove user-declared destructors and trivial copy ctor/assignment
implementations whose bodies are equivalent to the compiler-generated
defaults. The implicit special members are correct in every case (no
owning raw pointers, no side effects in the removed bodies, virtual
destructor inheritance preserved through base classes).
### utility/graph/ — non-owning iterator/element helpers
* `Graph.hh`: `EdgeListElement`, `EdgeListIterator`,
`EdgeListConstIterator` — drop user dtor and the `= default` copy ctor /
assignment.
* `Digraph.hh`: `DirectedEdgeListElement`, `DirectedEdgeListIterator`,
`DirectedEdgeListConstIterator` — drop user dtor and member-wise copy
ctor / assignment.
* `ArrayPool.hh` (`Array0`): drop user dtor, copy ctor, and
self-assignment-guarded copy assignment.
* `LowMemGraph.hh` (`LowMemGraphBase`): drop empty `override` dtor
(`utility::VirtualBase` supplies the virtual destructor).
* `UpperEdgeGraph.hh`: drop the empty `UEEdge` dtor and
`~UpperEdgeGraph() override = default;`.
`LowMemNode` / `LowMemEdge` are intentionally left untouched — their
explicit destructor declarations anchor a load-bearing comment about
deliberately not making the destructor virtual.
### utility/
* `DereferenceIterator.hh` (`DereferenceIterator`): drop empty user
dtor.
### utility/options/ — empty overriding destructors
Drop `~XxxOption() override {}` from the eight leaf scalar/vector option
classes. The implicit destructor is virtual via inheritance from
`Option` (which declares a virtual destructor), so dynamic dispatch is
preserved.
* `BooleanOption.hh`, `BooleanVectorOption.hh`
* `FileOption.hh`, `FileVectorOption.hh`
* `IntegerOption.hh`, `IntegerVectorOption.hh`
* `RealOption.hh`, `RealVectorOption.hh`
The remaining option classes with the same pattern (`PathOption`,
`StringOption`, `ResidueChainVectorOption`, `ScalarOption`,
`ScalarOption_T_`, `VectorOption`, `VectorOption_T_`) can be addressed
in a follow-up; the eight here form a coherent leaf-scalar-plus-vector
subset.
Improving `Pose.cache` dictionary getter and setter performance (#658)
This PR aims to improve the performance of `Pose.cache` dictionary data
accessors. Several code pathways run with O(N^2) (quadratic time
complexity) behavior, and new functionally equivalent fast data accessor
methods are introduced to run with O(N) (linear time complexity)
behavior:
- `Pose.cache.fast_items()`
- `Pose.cache.fast_values()`
- `Pose.cache.metrics.fast_items()`
- `Pose.cache.metrics.fast_values()`
- `Pose.cache.metrics.real.fast_items()`
- `Pose.cache.metrics.string.fast_values()`
- `Pose.cache.metrics.composite_real.fast_items()`
- `Pose.cache.metrics.composite_real.fast_values()`
- `Pose.cache.metrics.composite_string.fast_items()`
- `Pose.cache.metrics.composite_string.fast_values()`
- `Pose.cache.metrics.per_residue_real.fast_items()`
- `Pose.cache.metrics.per_residue_real.fast_values()`
- `Pose.cache.metrics.per_residue_string.fast_items()`
- `Pose.cache.metrics.per_residue_string.fast_values()`
- `Pose.cache.metrics.per_residue_probabilities.fast_items()`
- `Pose.cache.metrics.per_residue_probabilities.fast_values()`
- `Pose.cache.extra.fast_items()`
- `Pose.cache.extra.fast_values()`
- `Pose.cache.extra.real.fast_items()`
- `Pose.cache.extra.real.fast_values()`
- `Pose.cache.extra.string.fast_items()`
- `Pose.cache.extra.string.fast_values()`
- `Pose.cache.energies.fast_items()`
- `Pose.cache.energies.fast_values()`
Users must update their API calls to take advantage of these upgrades:
`dict(pose.cache)` -> `dict(pose.cache.fast_items())`, etc. These
improvements are only really noticable when there are hundreds to
thousands of scores cached in the `Pose.cache` dictionary. The basis for
the performance improvement is the following:
- `dict(pose.cache)` relies on `__iter__` (returns `pose.cache.all`) +
`__getitem__(key)` (returns `maybe_decode(pose.cache.all[key])`), where
it materializes the full scores dictionary for each key (O(N^2)).
- Instead, `dict(pose.cache.fast_items())` relies on simply `for k, v in
pose.cache.all.items(); yield k, maybe_decode(v)`, so the full scores
dictionary is materialized once for all keys (O(N)).
- It's also worth noting that the deprecated `Pose.scores` dictionary
(note `scores` not `cache`) has always performed with quadratic time
complexity (O(N^2)), and does not contain `Pose.scores.fast_items()` or
`Pose.scores.fast_values()` methods.
This PR also makes the `Pose.cache.all_scores` property run with O(N)
behavior, and removes an unnecessary argument from a private method:
`self._has_sm_data(pose)` -> `self._has_sm_data()`.
Additionally, this PR provides two new fast setter methods for mappables
(avoiding the relatively slow `Pose.cache.metrics` cleanup after each
item is set with `__setitem__`, and instead only performing one cleanup
at the end):
- `Pose.cache.metrics.real.set_mappable()`
- `Pose.cache.metrics.string.set_mappable()`
Micro-updates to the `PyRosettaCluster` interface are made to take
advantage of these performance improvements.