KC you were right, I think it wasnt liking me using new and old terminology, however it is now saying
number of arguments doesn't match prototype
So MK27, is on to something, however I thought the prototype was usually at the top of a document but I cant find it, the only time it is stated apart from being called, is
extern status NelderMeadSimplexMethod(n, f, xinit, length, fopt, timeout, eps, array1, array2)
yeah, that's a copy/paste missing bracket.
Oh I think I found the protoype in opt.h
I added the last two 'dbl' terms in but that doesnt fix it - It says the arrays dont match the protoypeCode:extern status NelderMeadSimplexMethod(int, dbl(), dbl *, dbl, dbl *, int, dbl, dbl, dbl);
error protype declaration, array1 does not match protoype...
Could it be because I am not giving the arrays a size?
I dont want to give them a size so ultimately the function can be used to solve more than one problemCode:extern status NelderMeadSimplexMethod(n, f, xinit, length, fopt, timeout, eps, array1, array2) //array1 and array2 allow two arrays to be passed to another function through NM int n; dbl (*f)(); dbl *xinit; dbl length; dbl *fopt; int timeout; dbl eps; dbl array1[], array2[];
The prototype should look more or less like the definition, without the parameter names:
Code:extern status NelderMeadSimplexMethod(int, dbl (*)(), dbl *, dbl, dbl *, int, dbl, dbl [], dbl []);
Last edited by anduril462; 08-17-2011 at 04:07 PM.
Throwing more arguments at a function isn't going to do anything. Even if it magically worked (let it compile), you still wouldn't actually be doing anything with those arguments in the function itself.Originally Posted by a.mlw.walker
Now look carefully at the above colors. Do all those types match?
Quzah.
Hope is the first step on the road to disappointment.
ahh thank you.
it compiles now but when i run it i get like a crash? I get a [file_name] is not working, progress bar thingy. what does this usually mean?
the function ultimately being called is
the main is calling:Code:static double function(int n, double x[], double Input[5], double Actual_Output[5])
NelderMead is initiated with, and declares the arrays like:Code:NelderMeadSimplexMethod(n, function, x, length, &fopt, timeout, eps, Input, Actual_Output)
the prototype in the header file isCode:extern status NelderMeadSimplexMethod(n, f, xinit, length, fopt, timeout, eps, array1, array2) dbl array1[], array2[];
and in main the arrays are given values here:Code:extern status NelderMeadSimplexMethod(int, dbl(), dbl *, dbl, dbl *, int, dbl, dbl[], dbl[]);
But it a little into the program running, I did a check, Actual_Output is making it to functionCode:Actual_Output[0] = 1.2; Actual_Output[1] = 2.693; Actual_Output[2] = 4.325; Actual_Output[3] = 6.131; Actual_Output[4] = 8.125; Input[0] = 1; Input[1] = 2; Input[2] = 3; Input[3] = 4; Input[4] = 5;
Last edited by a.mlw.walker; 08-17-2011 at 03:32 PM.
Quzah, loving the colours but i think they all match (I think...) i cant see one that doesnt but maybe im being blind, possibly array2[], but i think thats fine...
Quzah wrote that before you showed the (mostly) corrected code in post #65. I highlighted the 3 fixes in green in my post (#63). You missed the first one, for the function pointer parameter. Also, WTF is a dbl? The correct type in C is double. I certainly hope you didn't typedef that just to save a few keystrokes...
The red is a function call. The green is a function declaration. The bold stuff is some mixed up talk. Mixing up "words" (or nomenclature, or whatever you want to call it) is not trivial WRT programming because programming *is not math* it's language.
I think you are confusing the syntax for the two (call vs. declaration), and this could account for some of your other errors because of what I said in post #59 ("Unfortunately, the compiler is easily thrown off...").
A function call might initialize a variable, but it doesn't declare anything. A function declaration (prototype) does not initialize anything, and the only thing it declares is itself.
So that green thing needs some serious thought. As quzah pointed out, your parameters do not match the real prototype. As quzah did not point out -- one problem at a time, lol -- it is not a function call anyway, and if that is what you intended it to be, drop the "extern status" part. You call a function with its name. Its type has already been declared and by including that the compiler will treat the line as (an erroneous) declaration, not a call.
Last edited by MK27; 08-17-2011 at 04:23 PM.
C programming resources:
GNU C Function and Macro Index -- glibc reference manual
The C Book -- nice online learner guide
Current ISO draft standard
CCAN -- new CPAN like open source library repository
3 (different) GNU debugger tutorials: #1 -- #2 -- #3
cpwiki -- our wiki on sourceforge
ok the green line is the opening line of where the function is:
my language was wrongCode:extern status NelderMeadSimplexMethod(n, f, xinit, length, fopt, timeout, eps, array1, array2) int n; dbl (*f)(); dbl *xinit; dbl length; dbl *fopt; int timeout; dbl eps; dbl array1[], array2[]; { status stat = failure; int count, i; nvar = n; objective = f; initialize(); initial_simplex(xinit, length); /* fprint_simplex(stderr); */ for (i=0; i<=nvar; i++) { fvalue[i] = (*objective)(nvar, simp[i], array1, array2); } /* vectorfprint(stderr, nvar+1, fvalue); */ for (count=0; count<timeout; count++) { search_simplex(); compute_xcentroid(); /* fprint_points(stderr); */ compute_fmean_fvar(); /* fprintf(stderr, "fvar = %40.35f\n", fvar); */ if (fvar <= eps) { stat = success; break; } #if Debug if (count % SKIPTIME == 0) { fprintf(stderr, "k = %d f = %lg\n", count, fvalue[il]); /* vectorfprint(stderr, nvar, xinit); */ } #endif reflection(); if (freflect <= fvalue[is]) { if (freflect >= fvalue[il]) { vectorcopy(nvar, simp[ih], xreflect); fvalue[ih] = freflect; } else { expansion(); if (fexpand < fvalue[il]) { vectorcopy(nvar, simp[ih], xexpand); fvalue[ih] = fexpand; } else { vectorcopy(nvar, simp[ih], xreflect); fvalue[ih] = freflect; } } } else { if (freflect < fvalue[ih]) { vectorcopy(nvar, simp[ih], xreflect); fvalue[ih] = freflect; } contraction(); if (fcontract < fvalue[ih]) { vectorcopy(nvar, simp[ih], xcontract); fvalue[ih] = fcontract; } else { for (i=0; i<=nvar; i++) { if (i == il) continue; vectoradd(nvar, simp[i], simp[i], simp[il]); scalarvector(nvar, simp[i], 0.50, simp[i]); fvalue[i] = (*objective)(nvar, simp[i], array1, array2); } } } #if Debug /* fprintf(stderr, "%d : min = %lf\n", count, fvalue[il]); */ #endif } vectorcopy(nvar, xinit, simp[il]); *fopt = fvalue[il]; return stat; }
Okay, assuming "status" is some sort of typedef you created, then you want it to look like:
opt.h
neldermead.cCode:// prototoype - this is the only place you use extern extern status NelderMeadSimplexMethod(int, dbl (*)(), dbl *, dbl, dbl *, int, dbl, dbl [], []);
Code:status NelderMeadSimplexMethod(int n, dbl (*f)(), dbl *xinit, dbl length, dbl *fopt, int timeout, dbl eps, dbl array1[], dbl array2[]) { // your code here }
All apologies, that is an old school K&R style function definition, which quzah would recognize, so quzah was not leaving anything out.
EXCEPT, as anduril462 says, using extern with a definition is at best superfluous (and therefore confusing) and at worst just plain wrong. Where did you get this source from?
The purpose of extern is to indicate a function that is defined in an external object file to be linked in. You would use it in the way anduril462 indicated in post #70, but part of what's at issue here is how the project is "made". Do you have a makefile, or are you just calling the compiler directly (if you don't understand that, just explain what OS you are on, what compiler you are using, and how you access the compiler, eg, thru an IDE such as Code::Blocks)?
Last edited by MK27; 08-17-2011 at 05:31 PM.
C programming resources:
GNU C Function and Macro Index -- glibc reference manual
The C Book -- nice online learner guide
Current ISO draft standard
CCAN -- new CPAN like open source library repository
3 (different) GNU debugger tutorials: #1 -- #2 -- #3
cpwiki -- our wiki on sourceforge
Guys I have no more global variables. I have no more unused variables. It is clean (1 warning) C code.
Heres my
It runs and everything but its not getting the right values as before. However it may be readable nowCode:#include <stdio.h> #include <math.h> #include "opt.h" #include "stdlib.h" #define pi 3.141592653589793238462643 static double function(int n, double x[], double Input[5], double Actual_Output[5]) { double c; double Fitted_Curve[5] = {0}; double Error_Vector[5] = {0}; int i; double a, b, sum = 0; a = x[0];//parameters b=x[1]; c=x[2]; for (i = 0; i <= 4; i++) { Fitted_Curve[i] = (1/(a*b))*(c-asinh(sinh(c)*exp(a*Input[i]*2*pi))); Error_Vector[i] = Actual_Output[i]-Fitted_Curve[i]; printf("a(x[0]) = %f, b(x[1]) = %f, c(x[2]) = %f\n",x[0], x[1], x[2]); printf("Fitted_Curve= %f, Error_Vector= %f\n",Fitted_Curve[i], Error_Vector[i]); } for (i = 0; i <= 4; i++) { sum = sum + pow(Error_Vector[i],2); printf("sum= %f\n", sum); } return sum; } int main(void) { double Actual_Output[5]; double Input[5]; float tol; int n; //number of parameters to search for double x[3]; double length = 1.00; double fopt; int timeout = 1000000; double eps = 1.0e-12;//accuracy of Nelder Mead //Starting estimates for parameters a and b x[0] = 1;//a x[1] = 1;//b x[2] = -1;//c //Timing Values for 5 revolutions Actual_Output[0] = 1.2; Actual_Output[1] = 2.693; Actual_Output[2] = 4.325; Actual_Output[3] = 6.131; Actual_Output[4] = 8.125; //Revolution Number corresponding with revolution time above Input[0] = 1; Input[1] = 2; Input[2] = 3; Input[3] = 4; Input[4] = 5; n=3; //calculating physical condition parameters a and b if (NelderMeadSimplexMethod(n, function, x, length, &fopt, timeout, eps, Input, Actual_Output) == success) { printf("reaching to minimum "); } else { printf("timeout "); } printf(" [ %lf %lf %lf\n", x[0], x[1],x[2], fopt); return 0; }
Last edited by a.mlw.walker; 08-18-2011 at 04:06 AM.
Okay, your main.c looks much better, thank you.
I did all that, and everything compiled alright. Then I tried to run it, and it seg faulted. So I ran it through gdb. The green lines are commands I typed, the red stuff highlights the problems:Code:$ gcc -Wall main.c mat.c neldermead.c foo.c: In function ‘main’: foo.c:75: warning: too many arguments for format You need an extra %lf here for fopt foo.c:43: warning: unused variable ‘tol’ mat.c: In function ‘matrixinverse’: mat.c:344: warning: implicit declaration of function ‘exit’ mat.c:344: warning: incompatible implicit declaration of built-in function ‘exit’ You need to #include <unistd.h> or the Windows equivalent mat.c: In function ‘matrixallocfscan’: mat.c:489: warning: implicit declaration of function ‘malloc’ mat.c:489: warning: incompatible implicit declaration of built-in function ‘malloc’ You need to #include <stdlib.h> neldermead.c: In function ‘initialize’: neldermead.c:54: warning: implicit declaration of function ‘malloc’ neldermead.c:54: warning: incompatible implicit declaration of built-in function ‘malloc’ You need to #include <stdlib.h> neldermead.c: At top level: neldermead.c:27: warning: ‘fprint_simplex’ defined but not used neldermead.c:35: warning: ‘fprint_points’ defined but not used neldermead.c:43: warning: ‘fprint_generated_points’ defined but not used
So it was segfaulting on my system because the Input parameter to function was a bogus address. That came from the "fvalue[i] = (*objective)(nvar, simp[i]) line in NelderMeadSimplexMethod, which calls the "function" function via a pointer. objective is a function pointer, pointing to the "function" function. I printed the type of "function", and found that it takes 4 parameters, an int and 3 double pointers. But the call in NelderMeadSimplexMethod only provides the first two params. The other two are garbage values from the stack, resulting in undefined behavior. It seg faults on mine and looks like it kinda works on yours.Code:$ gdb a.out GNU gdb (GDB) Fedora (7.1-34.fc13) Copyright (C) 2010 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "i686-redhat-linux-gnu". For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>... Reading symbols from /home/charlesg/sandbox/test/matlab/a.out...done. (gdb) r Starting program: /home/charlesg/sandbox/test/matlab/a.out Program received signal SIGSEGV, Segmentation fault. 0x080486b1 in function (n=3, x=0x804c020, Input=0x3ff00000, Actual_Output=0x0) at foo.c:22 22 Fitted_Curve[i] = (1/(a*b))*(c-asinh(sinh(c)*exp(a*Input[i]*2*pi))); (gdb) bt #0 0x080486b1 in function (n=3, x=0x804c020, Input=0x3ff00000, Actual_Output=0x0) at foo.c:22 #1 0x08049e6f in NelderMeadSimplexMethod (n=3, f=0x8048624 <function>, xinit=0xbffff4a8, length=1, fopt=0xbffff4a0, timeout=1000000, eps=9.9999999999999998e-13, array1=0xbffff4c0, array2=0xbffff4e8) at neldermead.c:208 #2 0x080488bb in main () at foo.c:70 (gdb) up #1 0x08049e6f in NelderMeadSimplexMethod (n=3, f=0x8048624 <function>, xinit=0xbffff4a8, length=1, fopt=0xbffff4a0, timeout=1000000, eps=9.9999999999999998e-13, array1=0xbffff4c0, array2=0xbffff4e8) at neldermead.c:208 208 fvalue[i] = (*objective) (nvar, simp[i]); (gdb) up #2 0x080488bb in main () at foo.c:70 70 if (NelderMeadSimplexMethod(n, function, x, length, &fopt, timeout, eps, Input, Actual_Output) == success) { (gdb) ptype function type = double (int, double *, double *, double *)
Why didn't the compiler catch this? Because the function pointer you pass in is specified as dbl (*)(). Those empty ( ) at the end tell the compiler "this is a pointer to a function that takes 0 or more params of any type and returns a dbl". So it can't check. This is a trade-off in flexibility versus safe code. You can make this safer by making the param (in the prototype and definition) look like this: dbl (*f)(int, double *, double *, double *).
Make sure you pass 4 vars in the fvalue[i] = ... line, probably array1 and array2:
Code:fvalue[i] = (*objective) (nvar, simp[i], array1, array2);
Cool. Thats insane looking stuff you did to find the errors.
I have changed all the lines that call the function in the neldermead function to things like:
where ever it is called.Code:freflect = (*objective)(nvar, xreflect, array1, array2);
At the top of the file there is the line:
I changed this toCode:static dbl (*objective)();
Should I do it there also?Code:static dbl (*objective)(int, double *, double *, double *);
In fact it may be easier to give you all the files, because I have had to change stuff in all the files:
It should run for you like it does for me now
Yeah, the debugger was very intimidating for me at first, but learning to use it helped me not only find difficult errors, but write better code and understand what happens under the hood. A good debugger is, IMO, the most powerful tool a developer has.
Where is "there also"? You only mentioned one place. Basically, anywhere you declare or define (i.e. describe) a pointer to a function, you should be as specific as possible. The other two place I can think of are the prototype and definition of NelderMeadSimplexMethod:At the top of the file there is the line:
I changed this toCode:static dbl (*objective)();
Should I do it there also?Code:static dbl (*objective)(int, double *, double *, double *);
You'll have to fix any other function pointers yourself.Code:// prototype extern status NelderMeadSimplexMethod(int, int, double *, double *, double *), dbl *, dbl, dbl *, int, dbl, dbl[], dbl[]); // neldermead.c // FOR F***S SAKE, REMOVE THE extern KEYWORD FROM THE .c FILE status NelderMeadSimplexMethod(n, f, xinit, length, fopt, timeout, eps, array1, array2) int n; dbl (*f)(int, double *, double *, double *);
That big, bold red stuff is me yelling at you furiously. I don't particularly like being ignored (I told you 2 or 3 times, and some other people mentioned it too). I've explained to you exactly what the extern keyword does and where to use it and not use it. You've been told to read some books and tutorials, which should cover that as well. I'm done helping you until you fix your broken use of extern -- and I mean ALL incorrect uses of it, in every .c file. You really should remove ALL global variables, not just those in main.c. Another good practice you should implement (it's 3 simple lines of code) is to use include guards in each header file you create, even if you don't have a case for multiple inclusions (that may change later). Read this Wikipedia article: Include guard - Wikipedia, the free encyclopedia.
Runs, and I didn't get a seg fault, so that's good. The results still look off though (they match your screen shot in post #20).In fact it may be easier to give you all the files, because I have had to change stuff in all the files:
It should run for you like it does for me now
