Merge pull request #4262 from RosettaCommons/JackMaguire/StrongTypeAliases
Strong Sizes in JD3
`core::Size` is used frequently in JD3 (and Rosetta as a whole) to represent values that are conceptually different types but are all best represented with a simple `int`. Nobody should need to reinvent the `int` wheel every time we need a new integral type. `core::Size` is technically fine for all of these types but its ubiquitousness can lead to logic errors. The goal of this PR is to introduce distinct types for some common integral concepts in JD3. This will hopefully prevent bugs in JD3's already hard-to-understand framework.
Here is a good example of this feature being added.
Existing code:
```c++
StandardInnerLarvalJobOP create_inner_larval_job(
core::Size nstruct,
core::Size job_node,
core::Size preliminary_job_node
) const;
InnerLarvalJobOP
create_and_init_inner_larval_job_from_preliminary(
core::Size nstruct,
core::Size prelim_job_node,
core::Size job_node
) const;
```
These are two functions that JobQueen writers might replace with one another as drop-in replacements. If they do that, the compiler will not warn them that they need to _switch the 2nd and 3rd arguments_ because both arguments are core::Sizes. This logic error might not even be caught at runtime because `prelim_job_node` and `job_node` are often the same value for some JobQueens. The logic error might take months to reveal itself (like it did for MultistageRosettaScripts in #4238 ).
New code:
```c++
StandardInnerLarvalJobOP create_inner_larval_job(
core::Size nstruct,
JobDagID job_node,
PrelimJobNodeID preliminary_job_node
) const;
InnerLarvalJobOP
create_and_init_inner_larval_job_from_preliminary(
core::Size nstruct,
PrelimJobNodeID prelim_job_node,
JobDagID job_node
) const;
```
This new code requires minimal boilerplate (thanks to Rocco's suggestions in #4237 ) and would immediately alert developers _at compile time_ when they are using values incorrectly. Performance hits will not be a concern because the compiler completely optimizes this feature away, even in debug mode (see godbolt links in #4237 ).
notify author
notify list [rosetta-logs@googlegroups.com]
Merge pull request #4345 from RosettaCommons/vmullig/make_FASTER_ig_threadsafe
Add unit tests confirming that the FASTER interaction graph is threadsafe, and update test to use ref2015 instead of pre_talaris_2013_standard
The FASTER interaction graph unit test fails stochastically in the multi-threaded build. This is likely due to a thread safety issue. This PR attempts to fix the issue.
EDIT: The FASTER ig is, as far as I can tell, totally threadsafe. (This PR does add a pretty stringent test for this, computing the graph 100 times in threads and comparing each result to the graph computed in a single thread.) The score12 scorefunction, however, is not. I get concurrency issues ONLY when using score12, and these vanish with ref2015 (which is fully threadsafe). For now, I'm switching the FASTER ig unit test to use ref2015 instead of score12, though at some point I'll track down the concurrency issues in score12.
notify author
notify list [rosetta-logs@googlegroups.com]
Merge pull request #4350 from RosettaCommons/vmullig/fix_gcc_9_2_build
Fixing gcc 9.2.0 compilation.
A minor issue was introduced with the recent clang tidy clean-up. This PR replaces an explicitly-defined but trivial assignment operator with a default assignment operator to resolve the warning-as-error and to repair the build.
@roccomoretti
notify author
notify list [rosetta-logs@googlegroups.com]
Merge pull request #4336 from RosettaCommons/roccomoretti/clang_tidy_test_fixes2
Fix the `bugprone` series of clang-tidy tests.
This PR enables testing for (and fixing the existing cases of) the series of "bugprone" tests which clang-tidy tests for.
Mostly this was copy constructors which weren't calling their base class's copy constructor.
notify author
notify list [rosetta-logs@googlegroups.com]
Merge pull request #4341 from RosettaCommons/benchmark
Adding `--disable-new-dtags` linker option to default GCC settings, this should allow proper linking to Rosetta sqlite on Ubuntu.
notify author
notify list [rosetta-logs@googlegroups.com]
Merge pull request #4344 from RosettaCommons/vmullig/fix_gcc_9_2_for_scope_warning
Remove the -ffor-scope option from gcc 9.2 compilation. This flag is deprecated in newer gcc.
notify author
notify list [rosetta-logs@googlegroups.com]
Merge pull request #4331 from RosettaCommons/sergey/binder
- updating PyRosetta so it could be build with GCC-9
- updating Binder to use LLVM-6,
- fixing has_insertion_operator dependency cycle
- enabling bindings generation for abstract base classes even when they can not be derived-from in Python
notify author
notify list [rosetta-logs@googlegroups.com]
Merge pull request #4334 from RosettaCommons/roccomoretti/pyrosetta_zeromq_autoupdate
Update the PyRosetta build.py to automatically update the zeromq submodule.
If we need the zeromq module to build PyRosetta, make sure it's updated.
notify author
notify list [rosetta-logs@googlegroups.com]
Merge pull request #4089 from RosettaCommons/vmullig/multithreaded_packer_setup2
Multi-thread packer setup using the RosettaThreadManager work vector interface
This pull request replaces pull request #3964.
A current challenge is figuring out how to get all of the existing Rosetta protocols to take advantage of parallel processing, with as little burden placed on the user as possible. Much of the time in a typical Rosetta run is spent on:
- Computing interaction graphs (easy to parallelize).
- Running packing trajectories (hard to parallelize).
- Computing gradient vectors and inverse Hessian approximations for minimization (easy to parallelize).
- Computing the scoring function (somewhat hard to parallelize).
- Computing kinematics (may be parallelizable?)
This pull request aims to address the first point: it will allow interaction graphs to be calculated using the `RosettaThreadManager`'s thread pool, implemented in pull request #3946. The `RosettaThreadManager` is intended to allow us to experiment with parallelizing things on many different levels in Rosetta (from apps to job distributors to movers, down to low-level components like the packer, minimizer, and scoring machinery), without having things that call things that call things create thread explosions. (So, for example, Rosetta could launch 16 threads, the job distributor could request parallel job execution in 4 threads, and each parallel job thread could request up to 16 threads for packing, receiving 1 to 13 depending on availability so that the global pool never exceeds 16 running threads. Note that switching the `MultithreadedJobDistributor` to use the `RosettaThreadManager` will be for a future pull request.) It also allows very finely-grained threading, without having to encapsulate atomistic tasks in jobs that might have more overhead than the tasks themselves (as would be the case if we wanted to consider individual entries in the interaction graph calculation as "jobs").
Tasks:
- [ ] **FIX PYROSETTA DISTRIBUTED ISSUE**
- [x] Pull request #4310 must be merged before this one.
- [x] Pull request #3946 must be merged before this one.
- [x] Pull request #3957 must be merged before this one.
- [x] Add option (flag) for default number of threads that the packer will request.
- [x] Ensure that `fixbb` respects the option.
- [x] Test `fixbb`.
- [x] Ensure that the `PackRotamersMover` respects the option by way of TaskOperations.
- [x] Test `PackRotamersMover`. (Update test).
- [x] Ensure that the test checks that the single- and multi-threaded PDB output is identical.
- [x] Ensure that `FastRelax` respects the option by way of TaskOperations.
- [x] Ensure that `FastDesign` respects the option by way of TaskOperations.
- [x] Test `FastDesign`. (Update test).
- [x] Update the no-threads case in the `FastDesign` and `PackRotamersMover` tests.
- [x] ~~Modify `PackRotamersMover`:~~
- [x] ~~Add `MultithreadedMover` class that derives from `Mover` class.~~
- [x] ~~Have `MultithreadedMover` take an XML-configurable option for number of threads to request.~~
- [x] ~~Make the `PackRotamersMover` a `MultithreadedMover`, with its "number of threads" option overriding the global default if and only if provided by the user.~~
- [x] ~~Have the `PackRotamersMover` call the multithreaded `core::pack::pack_rotamers()` if number of threads is greater than 1.~~
- [x] Parallelize interaction graph setup.
- [x] Pre-identify (in a single thread) the one- and two-body interactions to be computed.
- [x] Pre-create the nodes and edges needed (using a single thread) in the interaction graph.
- [x] Handle the symmetric case, too -- need virtual function overrides in `SymmetricRotamerSets`.
- [x] Modify the interaction graph to allow parallel, threadsafe computation of elements.
- [x] Make a work vector of all onebody and twobody energies to compute, and pass it to the `RosettaThreadManager`.
- [x] Handle the lazy IG case -- probably not much that multithreading can do, but take a look anyway. --> Can only multithread the onebody energy precalculation. Meh. Better than nothing.
- [x] Integration/unit test the 1-thread case.
- [x] Integration/unit test the multi-threaded case.
- In the future, integration test multi-threaded packing in the context of multi-threaded job distribution.
- [x] Looks like I've got the same rare segfault issue. Track it down. --> Got it. Can't call functions that finalize edges from threads.
- [x] Integration test: confirm that interaction graphs computed in parallel are identical to interaction graphs computed in serial.
- [x] Symmetric case, too.
- [x] Move the `-packing:packing_threads` option to `-multi_threading:packing_threads` to keep all of these options together.
- [x] Same for the optH threads option.
- [x] Pilot app for performance testing.
- [x] Switch tracer output to debug output, or remove entirely.
- [x] Special-case the logic for 1 thread requested to use the non-threaded code.
- [x] Do this for `SymmetricRotamerSets`, too.
- [x] Fix the twobody energy calculation for the symmetric case.
- [x] Handle the case of threads requested in a non-threaded build.
- [x] **FIX PYROSETTA DISTRIBUTED ISSUE.**
- [ ] Developer documentation.
- [ ] User documentation.
- [x] Beauty.
TODO:
- [x] Make commandline flag more specific (`-interaction_graph_threads` instead of `packing_threads`.)
- [x] Take out new options from hydrate, FastRelax, FastDesign, PackRotamerMover, etc. for threads. Instead...
- [x] Add a new TaskOperation for setting number or threads.
- [x] PackerTask stores number of threads to request for interaction graph setup.
- [x] Maintain commutativity: taskop can only REDUCE the number of threads.
- [x] Have default behaviour be to use all available threads.
- [x] Switch integration tests to use this.
- [x] See note about Andrew's suggestion for removing code duplication; using work vector even in non-threaded build.
- [x] Allow thread manager to be called in threaded or non-threaded build.
- [x] Ensure that thread manager just executes the work that it is given (single-threaded) in the non-threaded build.
- ~~Add to correctness test at least one score value. (Especially in symmetric case.)~~ --> Already in unit tests.
- [x] Pull construction of temporary rotamer sets in symmetry code out of the precomputation and into the work vector work units (so that they don't persist).
- [x] Scrap the MultithreadedMover base class. For now, TaskOperations are the way to go; later, if we have multithreaded movers, we can add utility functions for their setup rather than having a base class for it.
At some point in the future:
- At some point, remove the default thread counts from `core/util/disulfide_util.hh`, find everything that fails to compile, and switch it all to explicitly request threads.
- At some point, remove the default thread counts from `core/pack/pack_rotamers.hh`, find everything that fails to compile, and switch it all to explicitly request threads.
- At some point, remove the default thread counts from `core/util/SwitchResidueTypeSet.hh`, find everything that fails to compile, and switch it all to explicitly request threads.
Vikram K. Mulligan 5 years Note: the unit test failure here is stochastic, and is due to lack of thread safety of the score12 scorefunction. Switching this test to ref2015 stabilizes it. At some point, I'll track down the issue in score12 that makes it non-threadsafe, but for now it's a low priority.
notify author
notify list [rosetta-logs@googlegroups.com]
Merge pull request #3575 from RosettaCommons/vmullig/update_dump_trajectory_energy
Allow easy visualization of packing trajectories
A while back, Brian Coventry added a clever means of visualizing minimization trajectories: the DumpTrajectoryEnergy. This is a scoring term that sneakily writes out a PDB whenever it is called to evaluate derivatives. This pull request modifies this scoreterm so that it can also dump out scoring trajectories (e.g. to visualize the _ab initio_ application's sampling) or packing trajectories.
Tasks:
- [x] Correct the access to the options system. (Use proper options constructor).
- [x] Add XML setup.
- [x] Add option for "stride", so that we don't have to dump every function evaluation, but could do, for example, every 10th or 100th or 100th.
- ~~Add separate strides for packing, minimizing, and scoring.~~
- [x] Add support for scoring.
- [x] Add support for packing.
- [x] Errors if we try to interrupt a packing or minimizing trajectory (since there _are_ end-of-trajectory signals).
- [x] Fix unit test failure.
- [x] Fix integration test failure.
- [x] Integration test for packing.
- [x] Documentation: added here https://www.rosettacommons.org/docs/wiki/scripting_documentation/RosettaScripts/RosettaScripts#rosettascript-sections_scorefunctions_a-utility-scoreterm-for-trajectory-visualization.
- [x] Beauty.
This pull request also adds some configuration for the VSCode IDE, to make it work a bit more smoothly with Rosetta's massive codebase. Users should be able to navigate to the Rosetta/main/source directory and simply type "code ." to have full editing capabilities, with Intellisense and all that good stuff.
Rocco Moretti 5 years This commit apparently perma-broke the duplicate_header_guards integration script, due to `INCLUDED_core_scoring_util_methods_DumpTrajectoryEnergy_hh`
notify author
notify list [rosetta-logs@googlegroups.com]