Most of this should be in now. If I missed anything, send in a (hopefully less monstrous) diff...
I'll send in the little bits I or you missed. Thanks.
Although everything compiles, MSVC spits out a bunch of warnings about the MKSUnit stuff; perhaps you could take a look and either satisfy yourself that this isn't a problem or (ideally) disambiguate things sufficiently so that it's happy:I looked this morning and, as far as I could tell, it was warning about using a standard feature of C++ that I was intentionally using. MSVC has ways to turning off specific warnings, so I may go and turn the warnings off for those calls.https://salilab.org/internal/imp/logs/imp/bin.i386-w32.log
I'll put this in shortly. Maybe I'm mistaken but it looked like NDEBUG didn't permanently disable the checks, just set the default check level. Most users would expect NDEBUG to disable the stuff at compile time. I'll check this to make sure that's really the case before it goes in.The patch is a bit hazy on what NDEBUG should do (reflecting my haziness). In general, NDEBUG should do or not do whatever we want it to do (and turning it on shouldn't make code slower). Other than that, I wouldn't worry too much about user expectations, especially since we don't allow the users to control it directly.
From what I understand, the tests were exposing bugs in IMP. The patches probably should have fixed the bugs and in case we should fix the bugs in IMP rather than just disable the tests. Or am I confused.Why would you want to do that? Those were bug fixes. The tests weren't failing - they were exposing bugs. By rolling back the changes, you reintroduce the bugs...
- there is code in restraints and things to return how much memory they used. I don't like this so much, since you add a virtual method to Object, which increases the overhead for everything. If you have to instrument, couldn't you just instrument the Restraint and/or ScoreState classes?Instrumenting the other base classes is probably sufficient. Anyway, the code is only occasionally useful, annoying to keep up to date and pretty much impossible to check meaningfully, which means it probably will end up incorrect. So dropping it is fine with me.