From fb1b184c8a5ca03ca71562d3f22c85a908e9024f Mon Sep 17 00:00:00 2001 From: Jack Maguire Date: Thu, 10 Oct 2024 09:24:47 -0400 Subject: [PATCH 1/8] Warning for bad patches --- source/src/core/chemical/ResidueType.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/source/src/core/chemical/ResidueType.cc b/source/src/core/chemical/ResidueType.cc index 78a9b1aa2d..bcf8ff5c25 100644 --- a/source/src/core/chemical/ResidueType.cc +++ b/source/src/core/chemical/ResidueType.cc @@ -62,6 +62,7 @@ // C++ headers #include +#include // std::stringstream #ifdef SERIALIZATION #include @@ -1150,6 +1151,16 @@ ResidueType::setup_atom_ordering( MutableResidueType const & mrt ) first_sidechain_hydrogen_ = attached_H_begin_[n_backbone_heavyatoms_+1]; } + if ( atom_order.size() != natoms_ ) { + std::stringstream ss; + ss << "atom_order.size() = " << atom_order.size() << '\n'; + ss << "natoms_ = " << natoms_ << '\n'; + ss << "This mismatch is expected to give a segfault down the line. The mismatch can be caused by:\n"; + ss << " - Using an invalid patch that has heavy/virtual atoms downstream of hydrogens.\n"; + ss << " - Maybe other reasons? Add them here if you find more please!"; + utility_exit_with_message( ss.str() ); + } + return atom_order; } From 66c74f24f61cfe94f8bb680118483de4aa07b2f3 Mon Sep 17 00:00:00 2001 From: Jack Maguire Date: Thu, 10 Oct 2024 09:28:42 -0400 Subject: [PATCH 2/8] Create bad_atom_ordering.patch Making a unit test out of the discussion from https://rosettacommons.slack.com/archives/C1YUVUD5E/p1728433183443519 --- .../core/chemical/bad_atom_ordering.patch | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 source/test/core/chemical/bad_atom_ordering.patch diff --git a/source/test/core/chemical/bad_atom_ordering.patch b/source/test/core/chemical/bad_atom_ordering.patch new file mode 100644 index 0000000000..5dbf6f40d1 --- /dev/null +++ b/source/test/core/chemical/bad_atom_ordering.patch @@ -0,0 +1,25 @@ +NAME AddVirtualAtoms +TYPES ADD_VIRTUAL_ATOMS +BEGIN_SELECTOR +# This patch can be applied to any residue type +END_SELECTOR +BEGIN_CASE +BEGIN_SELECTOR +# This case applies to all residues +END_SELECTOR +ADD_ATOM V0 VIRT VIRT 0.00 +ADD_BOND OG1 V0 +SET_ICOOR V0 1.00000000 1.00000000 1.00000000 OG1 CB CA +ADD_ATOM V1 VIRT VIRT 0.00 +ADD_BOND N V1 +SET_ICOOR V1 1.00000000 1.00000000 1.00000000 N CA C +ADD_ATOM V2 VIRT VIRT 0.00 +ADD_BOND O V2 +SET_ICOOR V2 1.00000000 1.00000000 1.00000000 O C CA +ADD_ATOM V3 VIRT VIRT 0.00 +ADD_BOND C V3 +SET_ICOOR V3 1.00000000 1.00000000 1.00000000 C CA N +ADD_ATOM V4 VIRT VIRT 0.00 +ADD_BOND 1HG2 V4 +SET_ICOOR V4 1.00000000 1.00000000 1.00000000 1HG2 CG2 CB +END_CASE From 092260fe551b35d3e81bc30fe5c6f9c0552802fa Mon Sep 17 00:00:00 2001 From: Jack Maguire Date: Thu, 10 Oct 2024 09:41:33 -0400 Subject: [PATCH 3/8] Created unit test for bad atom ordering based on this discussion: https://rosettacommons.slack.com/archives/C1YUVUD5E/p1728433183443519 --- .../core/chemical/ResidueTypeTests.cxxtest.hh | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/source/test/core/chemical/ResidueTypeTests.cxxtest.hh b/source/test/core/chemical/ResidueTypeTests.cxxtest.hh index d17823dba6..02b9ff4389 100644 --- a/source/test/core/chemical/ResidueTypeTests.cxxtest.hh +++ b/source/test/core/chemical/ResidueTypeTests.cxxtest.hh @@ -33,10 +33,15 @@ #include #include #include +#include #include +#include +#include + #include +#include #include @@ -657,4 +662,26 @@ public: } + void test_bad_atom_ordering(){ + bool reached_the_expected_point = false; + try{ + core::pose::Pose pose; + core::pose::make_pose_from_sequence( pose, "TESTMYIDEAPLEASE", "fa_standard" ); + core::Size resid = 4; + core::conformation::Residue const & r = pose.residue(resid); + core::chemical::Patch patch; + patch.read_file( "core/chemical/bad_astom_ordering.patch" ); + utility::vector1< core::chemical::VariantType > types; + patch.types( types ); + auto mutable_type = patch.apply( r.type() ); + reached_the_expected_point = true; + auto res_type = core::chemical::ResidueType::make( *mutable_type ); + TS_ASSERT(false); // the previous line should have thrown an error + auto new_residue = core::conformation::ResidueFactory::create_residue( *res_type, r, pose.conformation() ); + pose.replace_residue( resid, *new_residue, false ); + } + catch(...){} + TS_ASSERT( reached_the_expected_point ); + } + }; From 80e38de2e61248f353026902694bc6e18ad0f4c6 Mon Sep 17 00:00:00 2001 From: Jack Maguire Date: Thu, 10 Oct 2024 09:43:10 -0400 Subject: [PATCH 4/8] bad_atom_ordering.patch --- source/test/core.test.settings | 1 + 1 file changed, 1 insertion(+) diff --git a/source/test/core.test.settings b/source/test/core.test.settings index 1ecc2b4267..8689239ca9 100644 --- a/source/test/core.test.settings +++ b/source/test/core.test.settings @@ -760,6 +760,7 @@ testinputfiles = [ "chemical/atom_properties.txt", "chemical/ASX.params", # remove if the database version gets permanently enabled "chemical/LYX.params", # remove if the database version gets permanently enabled + "chemical/bad_atom_ordering.patch", "chemical/conflict_patch.txt", "chemical/conn.pdb", "chemical/carbohydrates/amylopectin_fragment.pdb", From cb021a9889835d9ffedd1969deaf16c1cf1ef314 Mon Sep 17 00:00:00 2001 From: Jack Maguire Date: Thu, 10 Oct 2024 09:46:11 -0400 Subject: [PATCH 5/8] typo fix --- source/test/core/chemical/ResidueTypeTests.cxxtest.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/test/core/chemical/ResidueTypeTests.cxxtest.hh b/source/test/core/chemical/ResidueTypeTests.cxxtest.hh index 02b9ff4389..ef66c26470 100644 --- a/source/test/core/chemical/ResidueTypeTests.cxxtest.hh +++ b/source/test/core/chemical/ResidueTypeTests.cxxtest.hh @@ -670,7 +670,7 @@ public: core::Size resid = 4; core::conformation::Residue const & r = pose.residue(resid); core::chemical::Patch patch; - patch.read_file( "core/chemical/bad_astom_ordering.patch" ); + patch.read_file( "core/chemical/bad_atom_ordering.patch" ); utility::vector1< core::chemical::VariantType > types; patch.types( types ); auto mutable_type = patch.apply( r.type() ); From c69d7f6787ba0679ca5faa05f8d6e432c62f5ff7 Mon Sep 17 00:00:00 2001 From: Jack Maguire Date: Fri, 11 Oct 2024 07:42:53 -0400 Subject: [PATCH 6/8] TS_ASSERT_THROWS_ANYTHING --- .../core/chemical/ResidueTypeTests.cxxtest.hh | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/source/test/core/chemical/ResidueTypeTests.cxxtest.hh b/source/test/core/chemical/ResidueTypeTests.cxxtest.hh index ef66c26470..9e0196e47a 100644 --- a/source/test/core/chemical/ResidueTypeTests.cxxtest.hh +++ b/source/test/core/chemical/ResidueTypeTests.cxxtest.hh @@ -663,25 +663,16 @@ public: } void test_bad_atom_ordering(){ - bool reached_the_expected_point = false; - try{ - core::pose::Pose pose; - core::pose::make_pose_from_sequence( pose, "TESTMYIDEAPLEASE", "fa_standard" ); - core::Size resid = 4; - core::conformation::Residue const & r = pose.residue(resid); - core::chemical::Patch patch; - patch.read_file( "core/chemical/bad_atom_ordering.patch" ); - utility::vector1< core::chemical::VariantType > types; - patch.types( types ); - auto mutable_type = patch.apply( r.type() ); - reached_the_expected_point = true; - auto res_type = core::chemical::ResidueType::make( *mutable_type ); - TS_ASSERT(false); // the previous line should have thrown an error - auto new_residue = core::conformation::ResidueFactory::create_residue( *res_type, r, pose.conformation() ); - pose.replace_residue( resid, *new_residue, false ); - } - catch(...){} - TS_ASSERT( reached_the_expected_point ); + core::pose::Pose pose; + core::pose::make_pose_from_sequence( pose, "TESTMYIDEAPLEASE", "fa_standard" ); + core::Size const resid = 4; + core::conformation::Residue const & r = pose.residue(resid); + core::chemical::Patch patch; + patch.read_file( "core/chemical/bad_atom_ordering.patch" ); + utility::vector1< core::chemical::VariantType > types; + patch.types( types ); + auto mutable_type = patch.apply( r.type() ); + TS_ASSERT_THROWS_ANYTHING( auto res_type = core::chemical::ResidueType::make( *mutable_type ); ) } }; From 35bd3c5082096b2282f591c9f1353e9276cf4848 Mon Sep 17 00:00:00 2001 From: Jack Maguire Date: Tue, 22 Oct 2024 20:31:13 -0400 Subject: [PATCH 7/8] Remove Whitespace --- source/src/core/chemical/ResidueType.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/src/core/chemical/ResidueType.cc b/source/src/core/chemical/ResidueType.cc index bcf8ff5c25..d5cc2265c7 100644 --- a/source/src/core/chemical/ResidueType.cc +++ b/source/src/core/chemical/ResidueType.cc @@ -1160,7 +1160,7 @@ ResidueType::setup_atom_ordering( MutableResidueType const & mrt ) ss << " - Maybe other reasons? Add them here if you find more please!"; utility_exit_with_message( ss.str() ); } - + return atom_order; } From b59a8ff2340887b29c56f25ced35d21970ac00ab Mon Sep 17 00:00:00 2001 From: Jack Maguire Date: Tue, 22 Oct 2024 20:32:37 -0400 Subject: [PATCH 8/8] tabs instead of spaces --- .../core/chemical/ResidueTypeTests.cxxtest.hh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/source/test/core/chemical/ResidueTypeTests.cxxtest.hh b/source/test/core/chemical/ResidueTypeTests.cxxtest.hh index 9e0196e47a..15c8c1494e 100644 --- a/source/test/core/chemical/ResidueTypeTests.cxxtest.hh +++ b/source/test/core/chemical/ResidueTypeTests.cxxtest.hh @@ -664,15 +664,15 @@ public: void test_bad_atom_ordering(){ core::pose::Pose pose; - core::pose::make_pose_from_sequence( pose, "TESTMYIDEAPLEASE", "fa_standard" ); - core::Size const resid = 4; - core::conformation::Residue const & r = pose.residue(resid); - core::chemical::Patch patch; + core::pose::make_pose_from_sequence( pose, "TESTMYIDEAPLEASE", "fa_standard" ); + core::Size const resid = 4; + core::conformation::Residue const & r = pose.residue(resid); + core::chemical::Patch patch; patch.read_file( "core/chemical/bad_atom_ordering.patch" ); - utility::vector1< core::chemical::VariantType > types; - patch.types( types ); - auto mutable_type = patch.apply( r.type() ); - TS_ASSERT_THROWS_ANYTHING( auto res_type = core::chemical::ResidueType::make( *mutable_type ); ) + utility::vector1< core::chemical::VariantType > types; + patch.types( types ); + auto mutable_type = patch.apply( r.type() ); + TS_ASSERT_THROWS_ANYTHING( auto res_type = core::chemical::ResidueType::make( *mutable_type ); ) } };