Rosetta Evolutionary Ligand (REvoLd) #303
Conversation
…ne and get rid of evolution option file if not needed
…ividual multiple times
…iptive error if that size is exceeded
… explain wait times
…n be parsed as score memory
Hackefleisch
added
ready_for_review
| @@ -0,0 +1,69 @@ | |||
| // -*- mode:c++;tab-width:2;indent-tabs-mode:t;show-trailing-whitespace:t;rm-trailing-spaces:t -*- | |||
There was a problem hiding this comment.
For organizational purposes, it might be worth putting the app in a subdirectory, rather than in the root applications dir. (Probably something suitably generic, like ligand_design.)
source/src/apps/public/revold.cc
Outdated
| #ifdef USEMPI | ||
| MPI_Init (&argc, &argv); | ||
| MPI_Comm_rank (MPI_COMM_WORLD, &rank); | ||
| MPI_Comm_size( MPI_COMM_WORLD, &size ); | ||
|
|
||
| if ( size < 10 ) { | ||
| TR.Warning << "Running REvoLd less than 10 CPUs causes very large runtimes. Consider using more CPUs." << std::endl; | ||
| } | ||
| #else |
There was a problem hiding this comment.
Beautifier doesn't necessarily run on #ifdef blocks -- you may need to adjust formatting manually. (Here use tabs instead of spaces.)
|
|
||
| if ( !basic::options::option[ basic::options::OptionKeys::ligand_evolution::options ].active() ) { | ||
| TR << "Path to evolutionary options file not set. Default settings will be used." << std::endl; | ||
| evoopt = EvolutionOptionsOP(new EvolutionOptions()); |
There was a problem hiding this comment.
(Minor) Using utility::pointer::make_shared() is preferred to directly calling new.
| #endif | ||
| } | ||
|
|
||
| std::string EvolutionManager::print_scores() const { |
There was a problem hiding this comment.
Slightly misleading name, as it doesn't actually print the score, just returns them as a string.
| } | ||
| } catch( ... ) { | ||
| // print message and continue scoring | ||
| TR.Error << "Unable to score " << library_.identifier_to_smiles( { ii, 0 } ) << std::endl; |
There was a problem hiding this comment.
Is the internal score annotation left in a reasonable state?
|
|
||
| /// @brief Maps selector names to types and parameter list | ||
| std::map< std::string, std::pair< std::string, std::map< std::string, core::Real > > > selector_options_ { | ||
| { "remove_elitist", { "elitist", { |
There was a problem hiding this comment.
Initializer lists don't have to be left-aligned. With such heavily nested data structures, you can (and probably should) indent them to be better readable.
There was a problem hiding this comment.
This was indented, but the beautifier squashed everything left. Is there a way to prevent it from doing so?
| } | ||
|
|
||
| Reagent::~Reagent() { | ||
| delete fingerprint_; |
There was a problem hiding this comment.
Probably better to use a std::shared_ptr (or std::unique_ptr) versus using manual memory management.
| // rdkit requires some weird vector and vector of vector shenanigans | ||
| RDKit::MOL_SPTR_VECT react; | ||
| for ( core::Size ii( 1 ); ii <= n_reaction_positions; ++ii ) { | ||
| react.push_back( RDKit::RWMOL_SPTR( reagents_[ reagent_indices[ ii ] ]->mol_ ) ); |
There was a problem hiding this comment.
reagents_[ reagent_indices[ ii ] ]->mol_ should already be a RDKit::RWMOL_SPTR, right?
| TR.Debug << "Len of outer vector: " << products.size() << std::endl; | ||
| TR.Debug << "Len of inner vector: " << products[ 0 ].size() << std::endl; | ||
|
|
||
| return RDKit::MolToSmiles( RDKit::RWMol( *products[ 0 ][ 0 ] ) ); |
There was a problem hiding this comment.
Is there a need to make a copy of the RWMol here? MolToSmiles should be able to work on the dereferenced ROMol directly.
| RDKit::ChemicalReaction* reac_; | ||
| core::Size possible_molecules_ = 1; | ||
|
|
||
| friend FragmentLibrary; |
There was a problem hiding this comment.
Try to avoid friend if at all possible. It took me way to long to understand where/how reagents_ was getting set -- simply having an "add_reagent()" function might better help encapsulate things and make the flow clearer.
| #include <protocols/ligand_evolution/offspring_factory/OffspringFactory.hh> | ||
|
|
||
|
|
||
| namespace protocols { |
There was a problem hiding this comment.
I'll note that you don't need to have a cc file if there's nothing to put in it. (Especially pure virtual classes can be header-only.)
This PR aims to include the newly developed application REvoLd into Rosetta.
REvoLd preprint: https://arxiv.org/abs/2404.17329
The algorithm optimizes small molecule ligands within combinatorial libraries like Enamine REAL. Most code changes are self enclosed and do not affect any functionality outside of the application. The only exception are changes to cmake compile directives for mpi which were broken.
A guide on how to run REvoLd is available here: https://docs.rosettacommons.org/docs/wiki/revold
Unit and integration test run locally and passed all.