Skip to content

Conversation

@robert-bolz
Copy link

This application builds on the existing SID_ERMS_prediction application (written by jtseffer). It simulates the ERMS data for an input structure, and rescores it based on the given experimental ERMS data. The rescored structures are given as the output (as well as the difference between the simulated and experimental ERMS data).

bolz.13 added 3 commits May 7, 2024 16:57
added it to apps.src.settings.all file, made two scripts in source to test and build said sid_erms app, and made an inputs directory in main to test the app.
@robert-bolz robert-bolz self-assigned this Jan 16, 2025
@roccomoretti
Copy link
Member

Looks like you've added an inputs/ top level directory and left some convenience scripts in the branch. (It also looks like you have two SID_ERMS_Rescore.cc -- I don't know if you need both.) Could you clean those up?

@robert-bolz
Copy link
Author

I just cleared the top level directory and deleted the second ERMS_app.cc. Thanks!

Copy link
Member

@roccomoretti roccomoretti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautification before merging is a must.

Copy/paste code duplication is also something to be addressed - please move the common code into protocols to share between the applications. (Feel free to make a new protocols subdir if none of the current ones fit -- just be general in naming, such that future loosely related work can go into there as well.)

Comment on lines 876 to 878
"pilot/robert_bolz" : [
"SID_ERMS_Rescore",
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to delete this if you've removed the pilot app.

basic::options::RealOptionKey const breakage_cutoff( "breakage_cutoff" );


//This section of code is copied and adapted from SID_ERMS_prediction (src/apps/public/analysis/SID_ERMS_prediction.cc) for the purpose of simulating the ERMS data for rescoring.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth extracting the shared code and putting it into the protocols level, rather than just copy-pasting it.

//end copied and adapted section

//calculate the rescored model
core::Real Rescore_models( const core::Real nscore_, const core::Real RMSE_, const core::Real max_, const core::Real min_ ){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rosetta convention is to use "east const": (core::Real const nscore_)

Also, it's best to reserve trailing underscores for class member variables -- 'regular' variables (including function parameters) should be "normal".

Comment on lines 535 to 537
std::stringstream err_msg;
err_msg << "Must input the complex type using the complex_type option." << std::endl;
utility_exit_with_message(err_msg.str());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just pass the string directly to utility_exit_with_message() (using a stringstream is only needed if you're doing some complex formatting.)


utility::vector1<std::string> files;
utility::vector1<file::FileName> list;
if ( option[in::file::l].user() ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably recommend using streams_from_cmd_line() from core/import_pose/pose_stream/util.hh -- this parses the command line and loads the poses for you. (And is flexible with various inputs: -s/-l/-in:file:silent/etc.)

MetaPoseInputStream input = streams_from_cmd_line();
core::pose::Pose current_pose;
while ( input.has_another_pose() ) {
    input.fill_pose( current_pose, *rsd_set );

    std::cout << "Read pose for file: " << core::pose::tag_from_pose(current_pose) << std::endl;
}

TR << "ERMS simulation complete" << std::endl;

//calculte RMSE (if applicable)
if ( option[ RMSE ].user() && option[ ERMS ].user() ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For boolean options, prefer value() to user()

(If someone sets -RMSE false on the command line, user() is true, but value() is false.)

Comment on lines 582 to 587
} else {
if ( !option[ basic::options::OptionKeys::in::file::l ].user() ) {
std::stringstream err_msg;
err_msg << "Must input either file containing B values (using flag B_vals) or PDB file to calculate B values for simulation." << std::endl;
utility_exit_with_message(err_msg.str());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to beautify -- that last curly brace isn't matched with the else clause, despite looking like it is from the indentation.

Comment on lines 637 to 646
for ( core::Size i = 1; i <= rmse_list.size(); i++ ) {
if ( max_rmse < rmse_list[i] ) {
max_rmse = rmse_list[i];
}
}
for ( core::Size i = 1; i <= rmse_list.size(); i++ ) {
if ( min_rmse > rmse_list[i] ) {
min_rmse = rmse_list[i];
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If found it interesting you used std::max_element above, but you're not using it (or std::minmax_element) here. (That's fine, I just found it interesting.)

bolz.13 added 3 commits February 12, 2025 15:42
protocols/sid_erms_prediction/sid_erms_simulate file, and then correctly
called those functions in the apps/public/analysis/SID_ERMS_Rescore.cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants