| View previous topic :: View next topic |
| Author |
Message |
Markdem
Joined: 24 Jun 2005
Posts: 206
|
| Working out percent with ints |
 Posted: Tue Jan 22, 2013 11:53 pm |
 |
|
Hi All,
I need to covert between 8bit int duty and a percentage and I need to round the results. I want to deal with ints to make the code as fast as I can, so I have come up with this:
| Code: |
int duty_to_percent(int duty)
{
int32 percent;
int fractional;
percent = ((int32)duty * 1000) / 255;
fractional = percent % 10;
if(fractional > 4)
{
percent++;
}
return percent / 10;
}
int percent_to_duty(int percent)
{
int32 duty;
int fractional;
duty = ((int32)percent * 255) / 100;
fractional = ((int32)percent * 255) % 100;
if(fractional > 4)
{
duty++;
}
return duty;
}
|
The code works, but is there anything I can do to improve it, make it faster or more efficient?
Thanks |
|
 |
Ttelmah
Joined: 11 Mar 2010
Posts: 19466
|
|
| Posted: Wed Jan 23, 2013 2:13 am |
|
|
Think for a moment about your numbers. You have a duty that can only be 0 to 255. You have turned this into a value from 0 to 100 with your sum:
percent = ((int32)duty * 1000) / 255;
and are then trying to use 4/5 rounding from the original duty number, to increment it by one digit. Think of the flow of numbers you get:
| Code: |
duty percent percent/rdg step final_percent
0 0 0
1 3 3 3 0
2 7 7 4 0
3 11 11 4 1
4 15 15 4 1
5 19 20 5 2
6 23 24 4 2
7 27 28 4 2
8 31 32 4 3
9 35 36 4 3
10 39 39 3 3
|
4/5 rounding on the original number, is not what you want/need. The steps in the integer maths occur at 1000/255 points, not at the '10' points, hence applying 4/5, results in an overlarge step at '5' where it comes into effect, and missing a step at 10 where it drops out of effect.
Better to simply 'move the value up before division'. Uses less maths too:
| Code: |
int duty_to_percent(int duty) {
int16 percent;
int fractional;
percent = (((int16)duty * 100)+49) / 255;
return percent;
}
|
You don't have to go to int32, and get rid of the modulus (slow operation),
and just add 0.49*100 to the integer before the division. Faster maths by a huge amount.....
Result is:
| Code: |
duty percent
0 0
1 0
2 0
3 1
4 1
5 2
6 2
7 2
8 3
9 3
10 4
|
You will find the numbers are nice and steadily distributed, and the maths is perhaps 3* faster.....
The same approach can be used on the other conversion too.
Best Wishes |
|
|
Douglas Kennedy
Joined: 07 Sep 2003
Posts: 755
Location: Florida
|
|
| Posted: Wed Jan 23, 2013 4:12 am |
|
|
Ttelmah's snippet
| Code: |
int duty_to_percent(int duty) {
int16 percent;
int fractional;
percent = (((int16)duty * 100)+49) / 255;
return percent;
} |
What if the divide was by 256 so a shift would do it. It introduces a 1/255 error or approx 1/4 percent lower result so if 49 is replaced by 74= 49+25 to compensate we would get about the same result and avoid the overhead of a division.
I didn't try this but it has a good chance of working. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19466
|
|
| Posted: Wed Jan 23, 2013 4:45 am |
|
|
Worth also just adding, that a simple 256 element const int lookup table, will be faster than either approach....
Best Wishes |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Wed Jan 23, 2013 6:15 am |
|
|
| Ttelmah wrote: | | Code: |
int duty_to_percent(int duty) {
int16 percent;
int fractional;
percent = (((int16)duty * 100)+49) / 255;
return percent;
} |
|
Be careful, the 'return percent;' is an int16 but the defined return value is an int8. This gives undefined behaviour. Same problem is present in the original Mark's code.
About adding the 49, that's an error and should be 127 or 128.
49 (or 50) would have been correct when dividing by 100.
Choosing 127 or 128 depends on whether you want to round down or round up in case of a tie break. For example, 23.5% will be rounded down with the '127' value. Using '128' instead will round up, so the 23.5 becomes a 24.
I tried to combined this with Douglas' suggestion for dividing by 256 and added 25. | Quote: | | It introduces a 1/255 error or approx 1/4 percent lower result so if 49 is replaced by 74= 49+25 to compensate |
Evaluating all results in Excel I found the combined output still to be too low. Problem turns out to be that the error is not approximately 1/4 percent but 0.4%. Full scale error is an offset of 100, so adding 100/2=50 gives more accurate results.
Combined this results into: | Code: |
int8 duty_to_percent(int8 duty) {
int8 percent;
percent = (int8) ((((int16)duty * 100)+128+50) / 256); // 128 for rounding. 50 for dividing by 256 instead of 255.
return percent;
} |
... or use the lookup table as suggested by Ttelmah. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19466
|
|
| Posted: Wed Jan 23, 2013 6:16 am |
|
|
Or, use :
| Code: |
int duty_to_percent(int duty) {
int16 percent;
percent = (((int16)duty * 104)+49);
return make8(percent,1);
}
|
This gives the effect of 4/5 rounding (+49) & saves division (using make8 to take the high byte). With a single int16 multiplication, it'll be slightly slower than the table lookup, but will be the smallest solution, and nearly the fastest, while being also close to mathematically correct as well. 
Best Wishes |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Wed Jan 23, 2013 6:36 am |
|
|
We both posted at the same time.
| Code: | | percent = (((int16)duty * 104)+49); |
Multiplying by 104 will give a wrong maximum percentage of 103 for duty cycle 255.
See my post above for the correct formula.
I don't have a compiler here. Perhaps Ttelmah's method for getting the high byte will be better optimized than my int8 cast.
Note that I didn't implement the 4/5 rounding as the original poster used the more common 5/6 rounding. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19466
|
|
| Posted: Wed Jan 23, 2013 8:48 am |
|
|
Yes. Decimal place here. 256/255, is 1.004. Not 1.04. Oops....
The error using 256, really is tiny.
The compiler should optimise/256 to a shift, and optimise this to taking the high byte, but using make8, is 'guaranteed' to get the high byte in a single fetch, without accessing the low byte at all. Easier to be safe on this one.
Best Wishes |
|
|
Markdem
Joined: 24 Jun 2005
Posts: 206
|
|
| Posted: Wed Jan 23, 2013 4:54 pm |
|
|
Thanks SO much guys.
I knew I was doing something wrong as the calculation was so long and complex.
Ttelmah, I did not even think of using a lookup table here. Good idea, I might end up using it.
Just for completion, for going the other way I now have:
| Code: |
int percent_to_duty(int percent)
{
int duty;
duty = ((int16)percent * 255) / 100;
return duty;
}
|
After a good nights sleep, looking back at the original code it make no sense to do any rounding. I have ran it thought the simulator and the numbers are looking perfect.
Thanks again.
Mark |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19466
|
|
| Posted: Thu Jan 24, 2013 12:44 pm |
|
|
Just worth saying that the result of returning an int16 variable, from a function returning an int8, is not 'undefined'. C explicitly casts the value from int16 to int8, and if the result is less than 255 (which with the maths as shown, it is), the cast always just returns the low byte. It _is_ undefined, if you mixed signed and unsigned types, but it is quite acceptable and 'defined' in the case used.
Best Wishes |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Thu Jan 24, 2013 2:41 pm |
|
|
| Ttelmah wrote: | | Just worth saying that the result of returning an int16 variable, from a function returning an int8, is not 'undefined |
Technically speaking you are right, but I don't like code with implicit 'smartness'. If you intend to return an int8, then add a cast to show so. It doesn't make the resulting program any larger but that way a future programmer who has a look at the code will see it was intentional behaviour instead of just a lucky coincidence. |
|
|
Markdem
Joined: 24 Jun 2005
Posts: 206
|
|
| Posted: Thu Jan 24, 2013 4:24 pm |
|
|
Thanks for the comments, they have been noted.
I will cast the result to int8 to make the function make more sense.
Thanks again.
Mark |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
