search

plumed / plumed2

Development version of plumed 2
https://www.plumed.org
GNU Lesser General Public License v3.0
364 stars 289 forks source link

Fixed some issues with regtests on secondary structure variables #1141

Closed gtribello closed 1 month ago

gtribello commented 1 month ago

This fixes some issues with the newer regtests on secondary structure variables. I have removed the test secondarystructure/rt-coverage-parabetarmsd-parabeta-intra as it is identical to rt33 with the exception of adding a secondary structure variable that is exactly zero because you are not considering any segments. I have also added checks in the constructors for AntibetaRMSD and ParabetaRMSD to ensure that the user has specified at least one segment to compare with the ideal structure.

For the other test that had to be fixed I specified the specific force values that I wanted to print to the output file and all problems were resolved.

Description

Addresses issues raised in #1137 @GiovanniBussi

Target release

I would like my code to appear in release 2.10

Type of contribution
Copyright
Tests
GiovanniBussi commented 1 month ago

Thanks @gtribello !

I have two questions:

p_both: SECONDARY_STRUCTURE_RMSD BONDLENGTH=0.17 STRUCTURE1=1.244,-4.62,-2.127,-0.016,-4.5,-1.395,0.105,-5.089,0.024,-0.287,-3,-1.301,0.55,-2.245,-0.822,-1.445,-2.551,-1.779,-1.752,-1.13,-1.677,-2.113,-0.55,-3.059,-2.906,-0.961,-0.689,-3.867,-1.738,-0.695,-2.774,0.034,0.19,-3.788,0.331,1.201,-3.188,0.3,2.624,-4.294,1.743,0.937,-3.503,2.671,0.821,4.746,-2.363,0.188,3.427,-1.839,0.545,3.135,-1.958,2.074,3.346,-0.365,0.181,4.237,0.412,0.521,2.261,0.013,-0.487,2.024,1.401,-0.875,1.489,1.514,-2.313,0.914,1.902,0.044,-0.173,1.33,0.052,1.202,2.94,0.828,0.19,3.507,1.718,0.772,3.801,3.104,-0.229,4.791,1.038,0.523,5.771,0.996 STRUCTURE2=-1.439,-5.122,-1.144,-0.816,-3.803,-1.013,0.099,-3.509,-2.206,-1.928,-2.77,-0.952,-2.991,-2.97,-1.551,-1.698,-1.687,-0.215,-2.681,-0.613,-0.143,-3.323,-0.477,1.267,-1.984,0.681,-0.574,-0.807,0.921,-0.273,-2.716,1.492,-1.329,-2.196,2.731,-1.883,-2.263,2.692,-3.418,-2.989,3.949,-1.433,-4.214,3.989,-1.583,2.464,-4.352,2.149,3.078,-3.17,1.541,3.398,-3.415,0.06,2.08,-2.021,1.639,0.938,-2.178,1.225,2.525,-0.886,2.183,1.692,0.303,2.346,1.541,0.665,3.842,2.42,1.41,1.608,3.567,1.733,1.937,1.758,1.976,0.6,2.373,2.987,-0.238,2.367,2.527,-1.72,1.684,4.331,-0.148,0.486,4.43,-0.415 ATOMS=7,9,11,21,22,23,25,27,37,38,39,41,43,53,54,55,57,59,69,70,71,73,75,85,86,87,89,91,101,102,103,105,107,117,118,119,121,123,133,134,147,149,151,161,162,163,165,167,177,178,179,181,183,193,194,195,197,199,209,210,211,213,215,225,226,227,229,231,241,242,243,245,247,257,258,259,261,263,273,274,287,289,291,301,302,303,305,307,317,318,319,321,323,333,334,335,337,339,349,350,351,353,355,365,366,367,369,371,381,382,383,385,387,397,398,399,401,403,413,414,427,429,431,441,442,443,445,447,457,458,459,461,463,473,474,475,477,479,489,490,491,493,495,505,506,507,509,511,521,522,523,525,527,537,538,539,541,543,553,554,567,569,571,581,582,583,585,587,597,598,599,601,603,613,614,615,617,619,629,630,631,633,635,645,646,647,649,651,661,662,663,665,667,677,678,679,681,683,693,694,707,709,711,721,722,723,725,727,737,738,739,741,743,753,754,755,757,759,769,770,771,773,775,785,786,787,789,791,801,802,803,805,807,817,818,819,821,823,833,834,847,849,851,861,862,863,865,867,877,878,879,881,883,893,894,895,897,899,909,910,911,913,915,925,926,927,929,931,941,942,943,945,947,957,958,959,961,963,973,974,987,989,991,1001,1002,1003,1005,1007,1017,1018,1019,1021,1023,1033,1034,1035,1037,1039,1049,1050,1051,1053,1055,1065,1066,1067,1069,1071,1081,1082,1083,1085,1087,1097,1098,1099,1101,1103,1113,1114,1127,1129,1131,1141,1142,1143,1145,1147,1157,1158,1159,1161,1163,1173,1174,1175,1177,1179,1189,1190,1191,1193,1195,1205,1206,1207,1209,1211,1221,1222,1223,1225,1227,1237,1238,1239,1241,1243,1253,1254,1267,1269,1271,1281,1282,1283,1285,1287,1297,1298,1299,1301,1303,1313,1314,1315,1317,1319,1329,1330,1331,1333,1335,1345,1346,1347,1349,1351,1361,1362,1363,1365,1367,1377,1378,1379,1381,1383,1393,1394,1407,1409,1411,1421,1422,1423,1425,1427,1437,1438,1439,1441,1443,1453,1454,1455,1457,1459,1469,1470,1471,1473,1475,1485,1486,1487,1489,1491,1501,1502,1503,1505,1507,1517,1518,1519,1521,1523,1533,1534,1547,1549,1551,1561,1562,1563,1565,1567,1577,1578,1579,1581,1583,1593,1594,1595,1597,1599,1609,1610,1611,1613,1615,1625,1626,1627,1629,1631,1641,1642,1643,1645,1647,1657,1658,1659,1661,1663,1673,1674,1687,1689,1691,1701,1702,1703,1705,1707,1717,1718,1719,1721,1723,1733,1734,1735,1737,1739,1749,1750,1751,1753,1755,1765,1766,1767,1769,1771,1781,1782,1783,1785,1787,1797,1798,1799,1801,1803,1813,1814,1827,1829,1831,1841,1842,1843,1845,1847,1857,1858,1859,1861,1863,1873,1874,1875,1877,1879,1889,1890,1891,1893,1895,1905,1906,1907,1909,1911,1921,1922,1923,1925,1927,1937,1938,1939,1941,1943,1953,1954,1967,1969,1971,1981,1982,1983,1985,1987,1997,1998,1999,2001,2003,2013,2014,2015,2017,2019,2029,2030,2031,2033,2035,2045,2046,2047,2049,2051,2061,2062,2063,2065,2067,2077,2078,2079,2081,2083,2093,2094,2107,2109,2111,2121,2122,2123,2125,2127,2137,2138,2139,2141,2143,2153,2154,2155,2157,2159,2169,2170,2171,2173,2175,2185,2186,2187,2189,2191,2201,2202,2203,2205,2207,2217,2218,2219,2221,2223,2233,2234,2247,2249,2251,2261,2262,2263,2265,2267,2277,2278,2279,2281,2283,2293,2294,2295,2297,2299,2309,2310,2311,2313,2315,2325,2326,2327,2329,2331,2341,2342,2343,2345,2347,2357,2358,2359,2361,2363,2373,2374,2387,2389,2391,2401,2402,2403,2405,2407,2417,2418,2419,2421,2423,2433,2434,2435,2437,2439,2449,2450,2451,2453,2455,2465,2466,2467,2469,2471,2481,2482,2483,2485,2487,2497,2498,2499,2501,2503,2513,2514 TYPE=DRMSD CUTOFF_ATOMS=6,21 STRANDS_CUTOFF=1.0

Can you confirm that this would not happen with the revised version of the code? (I can try as well, I was wondering if you took this into account when implementing the solution)

Thanks!

GiovanniBussi commented 1 month ago

@gtribello sorry for the double message.

First issue (why removing the test). I understand your point that, with your fix, the rt-coverage-parabetarmsd-parabeta-intra test does not make sense because it creates no segment and it then gives an error. Still, I am puzzled by the fact that this input was legal in v2.9 and now is wrong. Is this good or bad? I am not practical with these variables, maybe @carlocamilloni knows better?

(for Carlo: I think the point is if you define a region so short that you cannot have some secondary structure, should the code fail - as it does now - or give zero - as it did before; Gareth: I hope I summarized correctly)

On the second issue, I confirm. If I add that line to an input file I get a segfault. I think there's still some bug in SECONDARY_STRUCTURE_RMSD. Should the SEGMENT arguments be compulsory?

gtribello commented 1 month ago

@GiovanniBussi

In response to the first point in your first message, I think the fact that you could do it in earlier versions was an oversight on my part. I am pretty confident that the way I have fixed it is the right way. The CV measures the RMSD between a reference configuration for a beta sheet and each protein segment that could have a beta-sheet configuration. The CV then counts number of segments of protein that have a structure that resembles the reference beta sheet. If you say that there are no structures that could have a beta sheet structure then this CV is trivially zero so the result is strictly correct. However, if you are using a CV that probes beta-sheet formation in a protein that does not form beta sheets, what are you doing?

On the second point, I have just added a second commit that will ensure that inputs that don't specify the SEGMENT keyword for that action will crash properly.

GiovanniBussi commented 1 month ago

OK thanks I agree!