 |
 |
| View previous topic :: View next topic |
| Author |
Message |
xxopiumxx
Joined: 01 Jul 2011
Posts: 20
|
| Variable assignation problem |
 Posted: Tue Aug 23, 2011 7:57 am |
 |
|
Hi! Before anything, thanks a lot for your time.
I'm having the following problem, and I think it's as interesting as exasperating.
I'm working in a Vef meter, with a 10bits ADC.
I'm calculating a discrete integral, by performing the summation of the square of the ADC values, dividing it by the total amount of time that I have sum and taking its square root.
Something like this:
| Code: |
a=0;b=0;
for(i=1;i<=num;i++){
adc_v;
adc_v_d=adc_v - 512; //adjustment, because of the possible negative values, due the alter voltage
adc_v_d_c=adc_v^2;
a=b+adc_v_d_c; //summation, performed by accumulating a variable respect an auxiliary
b=a;
}
a_d=a/num;
a_d_sr=sqrt(a_d);
v_ef=a_d_sr;
|
adc_v must be a 16 bit integer,
adc_v_d a signed int 16 -because of the correction-,
adc_v_d_c must be a int32,
a_d must be float, in order not to lose the decimal values (am I wrong?, is there another way? like changing the scale of the dividend and then placing the comma manually?).
And finally, v_ef can be either int32, or int16 (the division and the square root reduce the max possible value into the int16 range).
The problem is that I don't just need accuracy but also speed. If I perform the variable assignation like I've described, it works fine on speed but fatal on accuracy (instead of 0,7v I'm getting 0,6V). But if I set ALL the variables as float (absurd, but, as a test) its precision is great (better than a 4 digits multimeter) but it takes an horrible amount of time. I mean it cannot take (at 2000 samples) the 10s that it is taking; it can be at max 2s.
So, the questions are:
(1) is changing from int to float is such a source of error? in that case is it the same error from int to float than from float to int?
(2) should I change the way in which I'm thinking the variable assignation in order to minimize the speed of processing without losing accuracy? or should I make some cast operation somewhere?
(3) or should I perform a hardware change, like getting a bigger crystal??
(4) I've noticed that if I do a^2 it takes more than if I do a*a (really, don't know why); but in the case of sqrt(a) I don't know how to avoid the normal expression and it feels like a giant waste of time, although totally necessary.
pd.: I'm working with a PIC16F877 with a 20mhz crystal, and the compiler is PCWHD 4.084 under Windows XP.
Last edited by xxopiumxx on Wed Aug 24, 2011 6:17 am; edited 1 time in total |
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Tue Aug 23, 2011 1:34 pm |
|
|
If you want us to work on this, post a small but compilable (without errors)
test program that demonstrates the problem. In other words, set it up
with some nominal input value to the calculation code (and tell us the
input value). Then tell us the output value that you get, and tell us the
result you would like to see.
If you do all this, then we can easily test it. |
|
|
xxopiumxx
Joined: 01 Jul 2011
Posts: 20
|
|
| Posted: Tue Aug 23, 2011 4:19 pm |
|
|
| PCM programmer wrote: | If you want us to work on this, post a small but compilable (without errors)
test program that demonstrates the problem. In other words, set it up
with some nominal input value to the calculation code (and tell us the
input value). Then tell us the output value that you get, and tell us the
result you would like to see.
If you do all this, then we can easily test it. |
Ok. Tomorrow I'll post the iteration code where I am having the problem. |
|
|
RF_Developer
Joined: 07 Feb 2011
Posts: 839
|
| Re: Variable assignation problem |
| Posted: Wed Aug 24, 2011 4:32 am |
|
|
OK, I can see you are not familiar with C. So let's take a look at your code:
| xxopiumxx wrote: |
Something like this:
| Code: |
a = 0;
b = 0;
for(i=1; i<=num; ++){
adc_v;
adc_v_d = adc_v - 512; //adjustment, because of the possible negative values, due the alter voltage
adc_v_d_c = adc_v^2;
a = b+adc_v_d_c; //summation, performed by accumulating a variable respect an auxiliary
b=a;
}
a_d = a/num;
a_d_sr = sqrt(a_d);
v_ef = a_d_sr;
|
(1) the fact of changing from int to float is such a source of error? in that case its the same error from int to float than from float to int?
|
Should be no error, floats can represent integers that are within their mantissa range precisely. That doesn't mean that arithmetic operations on those values will always be precise however, just that assignment of simple integers does not lose precision. That's the same with all languages on all computers. The range of integers for which this is true varies from one floating point representation to another. It will be different for floats and doubles on the same machine and same compiler for example.
| Quote: |
(2) should I change the way in which I'm thinking the variable assignation in order of minimize the speed of processing without losing accuracy? or make some cast operation somewhere?
|
Yes, casts are valuable and In this case essential. For example, adc_v - 512 is an operation on two unsigned 16 bit integers and will generate an unsigned result, which is not what you want and will sometimes generate serious errors, i.e. when adc_v is less than 512. To give what you want you need to cast adc_v to signed (assuming its in range of course): (signed int16) adc_v - 512. The result will then be the signed value you want. If you had written 512.0 then things would be even worse as that forces the computation to be done in floating point!
| Quote: |
(3) or should I perform a hardware change, like getting a bigger crystal??
|
If you're going to do a lot of floating point arithmetic then you need all the speed you can get, but you are already running your PIC at maximum speed.
| Quote: |
(4) I've noticed that if I do a^2 it takes more than if I do a*a (really, don't know why);
|
Simple. Raising to a power is done by logs in floating point. Your a^2 is (int32) (exp(pow((float)a) * 2.0f))) which takes a lot longer than a comparitively simple 32 bit multiplication. Most programming "101" courses tell you that a * a is always much faster than a ^ 2. So, do a * a.
| Quote: |
but in the case of sqrt(a) I don't know how to avoid the normal expression and it feels like a giant waste of time, although totally necessary.
|
Yes, there's no easy way round it. Its possible to code a mixed input float/output int version using any of the well known square root algorithms. That should be faster than the generic all float version provided by C but its never going to be "fast" on the PIC.
In general you'll always get more speed improvement by carefully considering the ranges and precision of the values and the arrangement of the algorithm than from anything else. Scaled integers are useful, for eaxmple voltages in my projects are integers in tenths of volts. I know you've already looked at this, but is there any way of, for example getting round the need for the square root?
If you post your actual code then we can do more, such as spotting any shortcuts and optimisations. We can't do a lot with psuedo-code. That said, a factor of 1000 speed up may well not be possible.
RF Developer |
|
|
xxopiumxx
Joined: 01 Jul 2011
Posts: 20
|
|
| Posted: Wed Aug 24, 2011 7:12 am |
|
|
Thanks a lot for the answer, it has been very handy.
I cannot post an input output results of the operation because it takes around 2000 samples to make it. but I'm always getting the same result with the same error, so I can post the C code of the iteration process and the result that it gives me in comparison of what should it.
particularly for RF_Developer:
| Quote: | Quote:
(2) should I change the way in which I'm thinking the variable assignation in order of minimize the speed of processing without losing accuracy? or make some cast operation somewhere?
Yes, casts are valuable and In this case essential. For example, adc_v - 512 is an operation on two unsigned 16 bit integers and will generate an unsigned result, which is not what you want and will sometimes generate serious errors, i.e. when adc_v is less than 512. To give what you want you need to cast adc_v to signed (assuming its in range of course): (signed int16) adc_v - 512. The result will then be the signed value you want. If you had written 512.0 then things would be even worse as that forces the computation to be done in floating point! |
Isn't there some auto-casting in that sort of operations? I mean, if I subtract 1024 to 512, for example, it wont give me -512 automatically in V_D considering that it is a 'signed int16' ???
Now, this is the code that is giving me the most significant errors. From it I must get around 0.72 and I'm getting 0.61 (**) . As you can see the value is totally coherent but, the error is inadmissible.
(**) after correcting the adc range: VR=Vef(2)*1024;
| Code: | int16 V;
signed int16 V_D;
int32 V_D_C,SumaAnterior,SumaTotal;
float Vef_d,Vef_d_r;
int16 Vef;
for(i=0;i<=nMax-1;i++){
set_adc_channel(0);
delay_us(10);
V=read_adc();
V_D=V-512;
V_D_C=V_D*V_D;
SumaAnterior=SumaTotal+V_D_C;
SumaTotal=SumaAnterior;
}
Vef_d=SumaTotal/nMax;
Vef_d_r=sqrt(Vef_d);
Vef=Vef_d_r;
Vef_LB=make8(Vef,0);
Vef_HB=make8(Vef,1);(***) |
(***)I'm doing this because I'm sending the result by serial communication.
the range of Vef fits in a int16 variable, and the discount of the square root mantissa isn't affecting the accuracy of the result; but: is it wrong to pass from variable to variable in order to reduce the size of the code? or should I make some cast?? (Sorry to ask it again but I haven't completely understood it).
pd.: I'm taking away the square root to another instance of the process, out of the PIC. |
|
|
RF_Developer
Joined: 07 Feb 2011
Posts: 839
|
|
| Posted: Wed Aug 24, 2011 10:09 am |
|
|
| xxopiumxx wrote: |
Isn't there some auto-casting in that sort of operations? I mean, if I subtract 1024 to 512, for example, it wont give me -512 automatically in V_D considering that it is a 'signed int16' ???
|
Yes, a lot of casting is automatic, but it may not be what you are expecting. In C the operands are automatically cast to match. To match the other operands, not the where the result is assigned to. In your example the operands are 1024 and 512, both unsigned int16. The operation (-) is therefore done in unsigned int16 arithmetic, giving 65024 (and an overflow but that's not trapped and you don't know it's happened) which is then cast to signed int16. The final cast may be OK, and even give the right answer, or the compiler may truncate the value into the largest positive value that will fit... you may end up with 32256! The compiler will do (signed int16)((int16) a - constant) when you need ((signed int16)a - (signed int16) constant). The two are not necessarily the same, and depends on whether the compiler is playing strictly by the rules, which CCS often doesn't.
I've been hit by this a number of times. For example when calculating temperature differences for a temp compensation algorithm.
Due to the same issue, Vef_d=SumaTotal/nMax will only give an integer result, despite Vef_d being a float. What you've got as a result of automatic casts is (float)((int32)SumaTotal/(int32)nMax) - integer division, result cast to float - when what you need is (float)SumaTotal/(float)nMax - float division, which is slower, but gives the precision you need. You don't have to explictly cast both, you only need to do one and the other will follow. So all you actually need to write is:
Vef_d = SumaTotal/(float)nMax;
or
Vef_d=(float)SumaTotal/nMax;
either will do OK.
I think a big part of your issues, about both speed *and* precision are a result of you making incorrect assumptions about how arithmetic is done. Understanding this stuff will allow you to get rid of all the intermediate variables as you will know what's going on. That will make your code clearer and easier to read. |
|
|
xxopiumxx
Joined: 01 Jul 2011
Posts: 20
|
|
| Posted: Thu Aug 25, 2011 6:37 am |
|
|
Thank you a lot. You have cleared lots of doubts that I had about casting, and arithmetical operations; which, at least for me, is even more important than get some code working.
I will set the castings as you have told me, and kick out from the PIC the square root.
Thanks again for your time! |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
