Pull Request №696 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/fix/incorrect-comparison-operators
Merge: d10f3af574b60550f256856d5a9742600ce1128b←453f38ad4e9ffd93c2be2fcabe2843fe3a16c9a8
Fix incorrect comparison logic in operator< / difference_from
----------------
Merge commit message:
Add comments explaining each comparison-operator fix
Brief one-paragraph notes above each of the four sites describing
the bug and why the corrected form is the right one, so a future
reader doesn't accidentally revert to the simpler-looking but
broken version.
Pull Request №696 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/fix/incorrect-comparison-operators
Merge: d10f3af574b60550f256856d5a9742600ce1128b←8fc631652510bbfa47c5575107e9de808b7804ce
Fix incorrect comparison logic in operator< / difference_from
----------------
Merge commit message:
Fix incorrect comparison logic in operator< / difference_from
Four independent classes had broken comparison logic that violated
either the contract of `operator<` (strict weak ordering) or the
intended semantics of `difference_from`:
* `core/chemical/sdf/mol_util.cc`: `BondData::operator<` was
`(lower < other.lower) || (upper < other.upper)`. This is not a
strict weak ordering — for `(5,1)` vs `(3,7)`, both compare less
than each other, breaking antisymmetry. `BondData` is used inside
`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(...)`, i.e. it
reported "less than" exactly when memcmp said "greater than" —
the comparison was inverted. Switched to `memcmp(...) < 0`, which
matches the correct convention (and matches `MotifHit::operator<`
in the same file).
* `core/io/StructFileReaderOptions.cc`: `operator<` was using `==`
instead of `!=` for the short-circuit "return false" lines. The
intent of the pattern is "if a < b return true; if a != b return
false; otherwise fall through to the next member" (this is exactly
what the parent `StructFileRepOptions::operator<` does), but the
`==` form returned false when members were equal — short-circuiting
before the rest of the members were ever compared, *and* failed to
return false when members satisfied `>`. 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 these fundamental properties *differ*, not when they match.
Otherwise, two types with equal charge but otherwise comparable
properties were misclassified as `Other` and the rest of the
function (which uses `charge_` to compute lone-pair / s-orbital
/ p-orbital differences) was unreachable. Fixed `==` to `!=`.
Rosetta Benchmark 12 hours Tests for this revision was automatically canceled because newer set of tests for pull-request №696 was submitted!
Pull Request №695 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/fix/off-by-one-pose-residue-loops
Merge: d10f3af574b60550f256856d5a9742600ce1128b←4b3d46d15a4d313462e17745dd631debe813c3b1
Fix off-by-one bugs in 1-indexed loops over pose residues
----------------
Merge commit message:
Fix off-by-one bugs in 1-indexed loops over pose residues
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 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 loop building the `RestrictResidueToRepacking` operation
skipped the last residue, so residues outside `new_positions`
at the C-terminus were designed instead of repack-only.
* protocols/fldsgn/BluePrintBDR.cc: same full-atom check bug as BDR.
* 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
trials was off by one (e.g. `bb_moves_=1000` actually ran 999).
Pull Request №694 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/utility-misc-empty-dtors
Merge: d10f3af574b60550f256856d5a9742600ce1128b←dea119ab102f68e70b46cf23cbadd6842c68bd31
Apply Rule of Zero across remaining utility/ empty destructors
----------------
Merge commit message:
Apply Rule of Zero across remaining utility/ empty destructors
Remove user-declared destructors and out-of-line `= default` definitions
where the implicit (or `= default`) destructor is correct, and clarify a
non-trivial destructor by deleting copy/move on its owner.
* Remove empty `~Foo() {}` / `~Foo() override {}` where the implicit
destructor is already correct (and virtual via base when needed):
- utility/Bound.hh, utility/excn/Exceptions.hh,
utility/io/ocstream.hh, utility/keys/AutoKey.hh,
utility/keys/UserKey.hh.
* Convert `~Foo() {}` to `~Foo() = default;` for polymorphic root
classes whose implicit destructor would otherwise be non-virtual:
- utility/Show.hh, utility/factory/WidgetFactory.hh,
utility/io/irstream.hh, utility/io/orstream.hh,
utility/keys/Key.hh, utility/options/Option.hh.
* Drop `= default` destructor pairs (.hh declaration + .cc definition)
for classes inheriting from utility::VirtualBase, which already
supplies a virtual destructor:
- utility/heap, utility/integer_mapping (subset_mapping),
utility/recent_history_queue, utility/io/GeneralFileManager
(GeneralFileContents{,Vector}), utility/tag/Tag.
* utility/io/mpistream.hh: explicitly `= delete` the copy ctor and
copy assignment of `basic_mpi_streambuf` because its destructor
calls `flush_final()` (closes the MPI channel); replace the empty
`~basic_mpi_ostream() override {}` with `= default`.
Pull Request №693 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/utility-options-empty-dtors
Merge: d10f3af574b60550f256856d5a9742600ce1128b←df1a70b0d08400256a9cb9cd9aee1fa7d0dfcf0b
Apply Rule of Zero across utility/options/ empty destructors
----------------
Merge commit message:
Apply Rule of Zero across utility/options/ empty destructors
Drop user-declared empty destructors from option and option-key classes.
The implicit destructor preserves virtual dispatch through inherited
virtual destructors (Option::~Option() and Key::~Key()).
utility/options/ — abstract bases and remaining leaf options:
AnyOption, AnyVectorOption, ScalarOption, ScalarOption_T_,
VectorOption, VectorOption_T_, PathOption, PathVectorOption,
StringOption, StringVectorOption, ResidueChainVectorOption.
utility/options/keys/ — base OptionKey plus 16 leaf key types:
Any/AnyVector, Boolean/BooleanVector, File/FileVector,
Integer/IntegerVector, Path/PathVector, Real/RealVector,
ResidueChainVector, Scalar, String/StringVector, Vector.
Follow-up to PR #692, which left these classes for a second batch.
Pull Request №692 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/utility-iterator-rule-of-zero
Merge: 80efd178b93d19b7cb76d9a0b34788104381afaa←b82b5545627e8bddf859f1c268b8e341296c449a
Apply Rule of Zero across utility/ helpers
----------------
Merge commit message:
Apply Rule of Zero across utility/ helpers
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:
* utility/DereferenceIterator.hh: empty user dtor.
* utility/graph/ArrayPool.hh (Array0): empty dtor plus member-wise
copy ctor and self-assignment-guarded copy assignment.
* utility/graph/Graph.hh / Digraph.hh: empty dtor and = default /
member-wise copy ops on EdgeListElement, EdgeListIterator,
EdgeListConstIterator and the Directed* counterparts (non-owning
raw pointers, default copy is correct).
* utility/graph/LowMemGraph.hh (LowMemGraphBase): empty override dtor;
base utility::VirtualBase already supplies a virtual destructor.
* utility/graph/UpperEdgeGraph.hh: empty UEEdge dtor and the
~UpperEdgeGraph() override = default declaration.
* utility/options/{Boolean,File,Integer,Real}Option(.hh) and their
*VectorOption.hh counterparts: empty ~XxxOption() override {}; the
base option classes' virtual destructors are still inherited.
Pull Request №684 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/metric-value-default-members
Merge: b8e64ecdc0338b77c231760f7f9bdcd4d500a240←e91f32a5c7a39c47bdcdb878354f61481e280532
Default MetricValue special members
----------------
Merge commit message:
Fix -Werror=unused-parameter on defaulted MetricValue copy constructor
CI build (gcc with -Werror=unused-parameter) flagged the named parameter
on the defaulted copy constructor as unused. Drop the name; the defaulted
body still copies the source.
Pull Request №691 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/key-containers-rule-of-zero
Merge: b8e64ecdc0338b77c231760f7f9bdcd4d500a240←9ee8c4c268c7bf060c2b014c3c94d2fca90685c5
Apply Rule of Zero to utility/keys container family
----------------
Merge commit message:
Apply Rule of Zero to utility/keys container family
ClassKeyMap, ClassKeyVector, KeyVector, SmallKeyMap, and SmallKeyVector each
held standard-container value-type members only (Vector, IndexMap, plus a
scalar Index in the Small variants). Their user-declared destructors
(empty body or = default), copy constructors, and copy-assignment operators
were all byte-for-byte equivalent to the implicit defaults that the compiler
would synthesize, so they were redundant.
Removing them lets the implicitly defaulted special-member-functions take
over and, as a side effect, restores the implicit move constructor and move
assignment that were previously suppressed by the user-declared copy
operations. The explicit default constructors on SmallKeyMap and
SmallKeyVector are kept because they value-initialize the scalar Index u_,
which an implicit default would leave indeterminate.
Pull Request №690 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/bitset-bitvector-rule-of-zero
Merge: b8e64ecdc0338b77c231760f7f9bdcd4d500a240←7f670472fa0942db38fbaf9eb58d619b79187a4d
Apply Rule of Zero to BitSet and BitVector
----------------
Merge commit message:
Apply Rule of Zero to BitSet and BitVector
Both class templates have a single value-type member (std::set / std::vector)
and no user-declared copy/move/assignment operators, so the explicit empty
destructors did nothing the compiler-generated ones wouldn't. They were also
suppressing the implicit move constructor and move assignment.
Remove the empty `~BitSet() {}` and `~BitVector() {}` declarations so the
classes follow the Rule of Zero and gain implicit move support.
Pull Request №689 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/disulfide-energy-iterators-rule-of-zero
Merge: b8e64ecdc0338b77c231760f7f9bdcd4d500a240←2da5ceb881f6c2016ec94af5cd47955000095865
Apply Rule of Zero to disulfide energy neighbor iterators
----------------
Merge commit message:
Apply Rule of Zero to disulfide energy neighbor iterators
The disulfide neighbor iterator classes in core/scoring/disulfides/
each had a user-declared destructor (defaulted in the .cc) and an
old-style private undefined copy assignment. The destructor did
nothing the compiler-generated virtual destructor wouldn't do, so
remove both the declaration and the empty `= default` definition.
Modernize the C++03-style copy-assignment prevention to public
`= delete`. The intent is preserved: assignment must go through the
polymorphic operator=(BaseType const &) override, which downcasts and
copies all members; the implicit derived-derived operator= would
bypass that path. A short comment captures this why.
Touched: CentroidDisulfideNeighborIterator/ConstIterator,
DisulfideMatchingNeighborIterator/ConstIterator,
DisulfResNeighbIterator/ConstIterator.
Pull Request №681 RosettaCommons/rosetta/main ← everyday847/rosetta/differentiable-parametric
Merge: cf532f89b6bbb7e67fd75031f3cc2aa95fb5ce19←21d90cb52d1abe277b363e833609842144201b21
Add differentiable parametric backbone minimization
----------------
Merge commit message:
Fix sidechain rebuild: use set_phi/psi/omega to sync atom tree
The previous approach (calling private/protected AtomTree methods)
didn't compile. Use public Pose::set_phi/set_psi/set_omega instead:
reading each backbone torsion from the new XYZ and writing it back
via set_torsion updates the atom tree's internal coordinates, which
triggers a coordinate rebuild that repositions all downstream atoms
including sidechains.
Rosetta Benchmark 1 week Tests for this revision was automatically canceled because newer set of tests for pull-request №681 was submitted!
Pull Request №681 RosettaCommons/rosetta/main ← everyday847/rosetta/differentiable-parametric
Merge: cf532f89b6bbb7e67fd75031f3cc2aa95fb5ce19←66ea3c3c82c9b9a8645bd6966946f26478975b02
Add differentiable parametric backbone minimization
----------------
Merge commit message:
Fix sidechain drift during parametric minimization, add trajectory dump
Sidechain fix: after setting backbone XYZ from Crick equations,
sync the atom tree's internal coordinates by reading phi/psi/omega
back from the new Cartesian positions and writing them as torsions.
Without this, the atom tree retains the old backbone frame and
sidechains don't follow the backbone when it moves.
Trajectory visualization: ParametricAtomTreeMultifunc can now dump
a PDB at each function evaluation via set_trajectory_dump(). Enabled
automatically when the AtomTreeMinimizer tracer is at Debug level
(-out:levels core.optimization.AtomTreeMinimizer:debug).
Rosetta Benchmark 1 week Tests for this revision was automatically canceled because newer set of tests for pull-request №681 was submitted!