Re: [IMP-dev] NBL cleanup and other small patches

[Ben Webb <>]

Daniel Russel wrote:
> > Although everything compiles, MSVC spits out a bunch of warnings about 
> > the MKSUnit stuff
> 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.

Good; that was my understanding too. It's unfortunate that MSVC sucks.

> 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.

The only thing we can say for sure is that when NDEBUG is defined, 
assert() is a noop. IMHO, IMP_assert() acts like assert(), so it should 
do the same thing - that's how I put it in the patch I committed. For 
the other runtime checks, it seems reasonable for it to be like CHEAP or 
NONE, as you say.

>  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.

You are probably confused because it did both. ;) The patch did fix the 
bug in IMP (use of invalidated iterators). It also disabled the particle 
deletion part of the tests, because IMP was touching freed particle 
pointers in that case. Arguably that's also a bug in IMP, but it was my 
understanding that we can't "fix" that without putting in particle 
reference counting, which I didn't have the time to do.

	Ben
--
                      http://salilab.org/~ben/
"It is a capital mistake to theorize before one has data."
	- Sir Arthur Conan Doyle