Fix GCC 16 (trunk) debug build: real bug + dead unused-but-set-variable cleanup (#728)
## Summary
This machine has GCC 12 through GCC 16 (16 is an experimental trunk
build) installed side by side. A survey of the debug library build under
each version found:
- **GCC 12, GCC 13**: build cleanly out of the box.
- **GCC 14, GCC 15**: need the `template-id-cdtor` fix in #723.
- **GCC 16**: needs #723's fix *plus* the changes in this PR.
Building under GCC 16 (with #723's fix applied) surfaced 16 unique
`-Werror` sites — mostly `-Wunused-but-set-variable`, plus two
`-Wmaybe-uninitialized` cases (one of which is a real bug):
1. **Real bug** — `core/chemical/CacheableResidueTypeSets.cc`: the copy
constructor initialized its base class with `CacheableData(*this)`
instead of `CacheableData(other)`, reading from the not-yet-constructed
destination object rather than the fully-constructed source. Harmless
today only because `CacheableData` has no data members of its own; still
wrong and exactly what GCC 16 is right to flag.
2. **False-positive trigger** —
`protocols/simple_moves/MissingDensityToJumpMover.cc`: the default
constructor called `MissingDensityToJumpMover::get_name()` (a qualified
call through `*this`, mid-construction) to build an argument for the
`Mover` base class. `get_name()` just returns a string literal, so it's
passed directly instead — avoids the pattern rather than working around
a compiler quirk.
3. **Dead loop counters** (13 sites across 9 files) — variables
incremented alongside a real loop iterator but never read anywhere:
`EnergyGraph.hh` (`iilag`, 2 of 4 occurrences — the other two are real
array indices and are untouched), `PDBInfo.cc` (`idx`, x2),
`mmtf_writer.cc` (`chainIndex`, `modelIndex`), `md.cc` (`imap`),
`StructureDataFactory.cc` (`cur_chain`), `FoldArchitectMover.cc`
(`count`), `pose_mod.hh` (`current_pos`), `DistanceScoreMover.cc`
(`ct_peaks`), `StructureDependentPeakCalibrator.cc` (`pose_ct`). No
behavior change — removed the tracking, left the actual iteration logic
untouched.
4. **Deliberately-unused, kept** — `SapConstraintHelper.cc`'s `offset`
is tracked "for symmetry" per an existing comment even though never
read. Rather than removing it against that stated intent, added an
explicit `(void)offset;` cast to satisfy the warning.
Drop template-id from ctor/dtor names in numeric/ (GCC 14/15 -Werror=template-id-cdtor) (#723)
## Summary
GCC 14+ enforces `-Werror=template-id-cdtor`: a class template may not
name its own constructors or destructors with explicit template
arguments — the injected-class-name must be used without `<...>`.
`numeric/MathVector.hh`, `numeric/MathMatrix.hh`, and
`numeric/histograms/OneDHistogram.hh` declared constructors (and, for
the first two, the destructor) in the disallowed form, e.g.:
```cpp
MathVector< T>() : ...
explicit MathVector< T>( const Size SIZE, ... ) : ...
~MathVector< T>() { ... }
OneDHistogram<key1>()= default;
```
Under GCC 14/15 in C++20 mode this fails to compile (`error: template-id
not allowed for constructor/destructor in C++20`). The `numeric/`
occurrences broke the plain library debug build before it could even
reach `basic/` or `core/`. The `OneDHistogram.hh` occurrence is more
subtle: its default constructor is only instantiated by a unit test
(`numeric/histograms/OneDHistogram.cxxtest.hh`), not by any library
code, so it slipped past a library-only build and only surfaced when
building/running the unit test suite — this is what broke CI on the
previous version of this PR.
This drops the `< T>` / `<key1>` from the constructor and destructor
declarator-ids, making them consistent with the copy constructors in the
same classes, which already use the correct injected-class-name form.
Return types, operators, and `new` expressions that legitimately use the
templated name are untouched. Pure syntactic correction, no semantic
change.
Verified with a clean `mode=debug` library build **and** `mode=debug
cat=test` unit-test build under GCC 15 (this machine's default), plus a
full library build under GCC 14 — both fully green. A targeted scan of
`source/src`, `source/test`, and `source/src/devel` for the same
declaration pattern turned up no other occurrences.
## Relationship to existing GCC 15 PRs (#555, #556)
This is not the first attempt at the GCC 15 build. Two earlier community
PRs are still open and overlap with this one:
- **#555 — "Fix C++20 template-id errors in constructors/destructors"**
(@saberger). This contains the **identical**
MathVector/MathMatrix/OneDHistogram template-id fix as this PR, and
additionally covers `core/scoring/lkball/LK_DomeEnergy.cc`,
`protocols/forge/remodel/RemodelGlobalFrame.cc`,
`utility/options/VectorOption_T_.hh`, two unit tests, and
`tools/build/basic.settings` — none of which have this same
template-id-cdtor pattern currently (checked directly), so those are
addressing separate GCC 15 issues.
- **#556 — "Fixes for compiling with gcc15"** (@roccomoretti). The
broader assorted GCC 15 fixes. Does not touch these three files.
Because #555 already lands the same fix, #723 is largely redundant with
the `numeric/` slice of that effort. It's offered as a minimal,
narrowly-scoped version of just the `template-id-cdtor` correction in
case it's useful to merge the build-blocking part independently;
otherwise #555 (combined with #556, per its author's note) supersedes
it. Closing this in favor of #555 + #556 is fine if the maintainers
prefer to consolidate the GCC 15 work.
## Note on diff size
This is a small change (three files, ~18 lines) — below the usual
bundling threshold — but it is the complete fix for this pattern across
the codebase: a scan of `source/src`, `source/test`, and
`source/src/devel` found no other occurrences of a class template naming
its own constructor/destructor with a template-id. Kept narrowly scoped
to this one toolchain-compatibility pattern.
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'
```
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 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.
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.
Refactor ozstream raw pointers to unique_ptr; modernize orstream copy ops (#667)
## Summary
- Replace three raw owning pointers in `ozstream` (`char_buffer_p_`,
`zip_stream_p_`, `mpi_stream_p_`) with `std::unique_ptr`, eliminating
manual `delete`/`nullptr` pairs in `open()`, `open_append()`,
`open_append_if_existed()`, `close()`, and the buffer helpers
- Modernize the private-undefined copy constructor/assignment in
`orstream` to explicit `= delete`, consistent with the approach used in
`irstream`/`izstream` (PR #664)
## Test plan
- [x] Full debug build passes (`python3 ./scons.py -j16 mode=debug`)
- [ ] No behavior change expected — ownership semantics are identical;
`close()` still runs `zflush_finalize()` and
`mpi_ostream::close()/clear()` before releasing the pointers
Refactor izstream raw zip_stream_p_ pointer to unique_ptr (#664)
## Summary
- `izstream` held a raw owning `zlib_stream::zip_istream*` managed by a
custom destructor, with copy/assignment only blocked by inherited
`private` declarations in `irstream` — no explicit policy on `izstream`
itself
- Replace `zip_stream_p_` with
`std::unique_ptr<zlib_stream::zip_istream>`, eliminating the manual
`delete` in the destructor
- Explicitly `= delete` copy constructor and copy assignment directly on
`izstream`
- Update all allocation/reset sites in `izstream.hh` and `izstream.cc`
to use `unique_ptr::reset()`
## Test plan
- [x] `utility/io` builds cleanly in debug mode
- [x] Full debug build passes with no errors
Updating documentation and typing for PyRosettaCluster (#646)
This PR adds several improvements to the _PyRosettaCluster_ framework.
### Major changes (maintaining runtime functionality)
1. Improve formatting and clarity of docstrings for Sphinx-based
PyRosetta documentation.
2. Consolidate typing aliases into a single
`pyrosetta.distributed.cluster.type_defs` module for easier
maintainability.
3. Add typing to attributes in `attrs` classes.
4. Clean up imports and typing (preserving Python-3.8 compatibility)
5. Add `pickle` and `cloudpickle` warnings to relevant docstrings.
6. Clarify and format logging and error messages.
7. Add `PackedPoseHasher` and `secure_read_pickle` to the top-level
`pyrosetta.distributed.cluster` namespace for easier usability.
### Minor changes (updating runtime functionality)
8. Update usage of `dataclasses` module to use the `attrs` package
instead. Also package Dask task arguments in a new `ExtraArgs` `attrs`
class (with slots and typing) instead of a simple dictionary.
9. Slightly loosen a validation: test whether two Base64-encoded pickled
`Pose` objects are identical -> test whether the scientific state of two
`Pose` objects are identical.
10. Handle edge case of single dictionary outputs from PyRosetta
protocols decorated with the `reserve_scores` decorator.
11. Handle edge case of unordered iterables (i.e., `set` objects) and
raise exceptions: input PyRosetta protocols, and outputs produced by
PyRosetta protocols, must be ordered for reproducibility purposes.
12. Fix exception handling for `Exception` rather than `BaseException`.
Update SecureUnpickler disallowed packages (#611)
This PR updates the `pyrosetta.secure_unpickle.SecureUnpickler` class to
block some additional callable targets via the `pickle` module,
including `numpy.load` and `pandas.read_pickle` modules. Unit tests
added herein demonstrate that secure `numpy`/`pandas` modules like
`numpy.array` and `pandas.DataFrame` are still deserializable.
Updated PARCS applications and IMMS_CCS score function (#609)
This application builds on the existing PARCS (parcs_ccs_calc.cc)
application and the IMMS_CCS energy term originally developed by
smturzo.
I extended PARCS to support multimeric protein complexes, enabling
simulation of PARCS CCS data for input structures containing multiple
chains. In addition, the existing IMMS_CCS energy term, which was
previously limited to monomers, was generalized for complexes through
the introduction of a new IMMS_ComplexCCS_Energy term. I also
implemented a new CCS_IMMS_with_CryoEMEnergy score term that integrates
experimental CCS data with cryo-EM information.
Method: For PARCS multimer support, I introduced a boolean flag
(-multimer) to the existing PARCS application. When enabled, the
algorithm predicts CCS values for multimeric assemblies by
reparameterizing the original CCS calculation.
For the IMMS-based energy terms, I developed on the existing
CCS_IMMSEnergy implementation
(source/src/core/energy_methods/CCS_IMMSEnergy.cc/.hh) by adding new
energy classes:
* CCS_IMMSComplexEnergy, which enables CCS-based scoring for protein complexes
* CCS_IMMS_with_CryoEMEnergy, which incorporates cryo-EM restraints alongside experimental CCS data
Integration test: I did integration just like how it was done for
monomers with additional flag. for multimer The test passed.
Fix dropped settings issues in HighResDocker (#520)
The copy constructor of HighResDocker was not copying over the resfile_
member, which means it was ignoring that setting. Since the copy
constructor is effectively a straight member-by-member copy, we can
simply delete it and rely on the autogenerated copy constructor.
Additionally, I noticed that the initialize_from_options() function was
declaring local variables, rather than changing the member variables.
Fix this.
Supporting task retries in PyRosettaCluster (#605)
`PyRosettaCluster` supports running tasks on available compute
resources; however, often it's more economical to run tasks on
preemptible compute resources, such as cloud spot instances or backfill
queues. This PR exposes Dask's task retry API via the
`PyRosettaCluster.distribute` method, allowing configuration of the
number of automatic retries for each submitted task. When the `retries`
keyword argument parameter is set, `PyRosettaCluster` will reschedule
failed tasks up to the specified number of times if compute resources
are reclaimed midway through a protocol.
This PR also adds a logging warning if using the `resources` keyword
argument with `dask` version `<2.1.0`.