Problems with NR ver 3
Chris2
09-26-2007, 08:00 AM
I have just started to use NR 3.01. First let me say thanks for the book and the code -- I appreciate the new functions, the more object-oriented code, and the use of functors for passing functions.
I have found a few issues.
1. If I define _USERNRERRORCLASS_, and include nr3.h from more than one of my cpp files, I get linker errors complaining that NRcatch is defined multiple times. You need to add "inline" to the definition of NRcatch to avoid this.
2. If I include any of the header files that define global functions from more than one of my cpp files (eg, fourier.h), I also get linker errors complaining that the global functions are defined multiple times. You need to add "inline" to the global function definitions.
3. The file nr3.h defines a macro for "throw(message)". This breaks my existing code! If I include nr3.h in my cpp file, you have redefined what throw means in my code. And redefining a keyword to mean something else is bound to cause confusion when reading the code. I suggest you define and use a new macro, perhaps NR_THROW, to throw exceptions within the numerical recipes code. Then I can redefine this macro in nr3.h to do what I want.
4. The file nr3.h includes a "using std;" statement. Again, this can break my existing code. And it's unnecessary, there are just a few places in nr3.h that use STL constructs; it's easy to use the explicit std:: namespace qualifier.
I'm working in VC++ 2005, but I believe the above issues affect all compilers.
Chris
Bill Press
09-26-2007, 02:54 PM
Chris,
These are all excellent points. Your posting as an "early adopter" should certainly help other people. Let me comment on the points in turn, because there may be things that I don't understand and that you could further clarify.
1. The include file nr3.h has #ifndef _NR3_H_ (etc.) directives that automatically prevent it from being included more than once in a single compilation unit, so in such a case it does no harm to include it more than once. I conclude that you must be dealing with the case of more than one separate compilation unit. I think we'd recommend putting *all* your included NR3 code in a single compilation unit if possible. In fact, these days, compilers are so fast that probably 99% of all users can put their whole project into a single compilation unit.
2. The majority of the NR3 .h files contain (global) function definitions, not just declarations. These are definitely intended to be included only once and in a single compilation unit. We *don't* use the convention that .h files include declarations only, with .c files including the definitions. The reason is that, with some classes or functions templated and others not, this distinction becomes very blurry and is hard to be consistent about. The best way to figure out the order in which to include any number of NR3 routines in a single compilation unit is to use the code dependencies tool at http://www.nr.com/dependencies/.
3. Our intention is that any user who is seriously using throw...catch should either redefine, or else eliminate, the throw() macro in nr3.h . I think you can always redefine it so as not to break your code: preprocessor macros never recursively expand their own name, so you should be able to define the throw() macro to throw your own error class (as is done now for the NRerror class). At the worst, you might need to define a "wrapper class" that has constructors to handle both your usage and for the throw("string") usage in the NR3 code. It would be interesting if you could post a description of your error class, so that we can figure out if this is true!
4. I think you are right that "using std" can be removed from nr3.h.
Cheers,
Bill P.
Chris2
09-28-2007, 04:21 PM
Thanks for breaking out the version 3 stuff to new forums!
Re: (1): It depends on what you mean by a "compilation unit". I think you mean a single cpp file. If that's the case, then no, I can't put all the code that uses NR into a single cpp file. To do so I would basically have to wrap all the NR code in another set of objects or functions so I can use them in the places in my code that need them.
I would strongly disagree with "99% of all users can put their whole project into a single compilation unit." It has nothing to do with how fast the compiler is. It has to do with trying to read and understand the code.
On the other hand, if "compilation unit" means a single project, then that doesn't fix the problem.
To be clear, my problem is when I have a project with two cpp files, say Source1.cpp and Source2.cpp. Both of them include nr3.h.
This will compile fine. But when you go to link, you will get linker errors, because both Source1.obj and Source2.obj will define (contain) the function NRcatch.
If this isn't clear enough, let me know and I'll create a trivial example project that demonstrates the problem.
This also applies to (2). Say both Source1.cpp and Source2.cpp use, say, realft, and #include fourier.h. There's no error at compile time, because I'm not including fourier.h twice in any one cpp file. But at link time, I'll get doubly defined errors.
Note that adding "inline" to the global definitions doesn't hurt anything. The compiler won't really inline them unless it makes sense to do so. The reason to add "inline" is to tell the compiler that it shouldn't complain about multiple definitions of the same routine.
I think this can produce some code bloat, because the compiler might include two copies of a function, but the only alternative to that is to, as you say, have a mixture of .h and .cpp files, and this can get messy.
As for (3), let's say that in my code I am throwing and catching strings in my own code. Now I include your nr3.h in my cpp file. I don't define _USENRERRORCLASS_, I don't even think about how NR is handling errors. I don't even use any NR classes or functions. But by including nr3.h, suddenly my throw statements aren't throwing string any more, they are throwing the integer 1 (because that's what your default macro does).
Sure, I can fix nr3.h to do the right thing, but I think nr3.h shouldn't be messing with basic language constructs.
Also, remember that in C++, I can write either:
throw(message);
or
throw message;
The first will will invoke your macro, and the second will not (or it will generate an error if your macro is defined, I forget which). This kind of thing makes reading and debugging code much more difficult. In fact, any time you redefine the meaning of what people think they already understand, you make code more difficult to read.
Let me say that I consider these minor issues. I have found the version 3 algorithms to be your usual high quality, easy to use, and the change to templates and a more object oriented approach is great. The new book is very nicely done, as well. I don't want my appreciation for what you've done to get lost among these small criticisms!
Chris
nekkceb
08-02-2009, 10:06 PM
Was this resolved? how do you deal with the linker errors in a project with multiple .cpp files that need to refer to elements in NR3? I have tried putting all the includes in one .h file, but because that file gets included in multiple .cpp (or the .h files for those), it still generates the linker error (each .cpp compiles just fine). Are there any tricks to get around this?
Using windows, visual C++, ver 7.1 (I think that is the same as 2003...)
nekkceb
08-03-2009, 09:56 AM
Just a clarification, I meant is there any solution that does not require editing the NR source? I regard that as troublesome, I would have to re-edit with each upgrade of NR source, unless:
Is there plan to deal with the issues Chris raised in future releases?
BTW i whole-heartedly agree with Chris, most folks can not put all code into one compilation unit. I use the Qt framework pretty heavily, which means each dialog or analysis routine is compiled as a separate .cpp/.h, which produces .obj's that the linker has to knit together for the final .exe or .dll.