Page MenuHomePhabricator

eina_matrix: replace cosf by cos to gain more accuracy
ClosedPublic

Authored by akanad on Nov 19 2019, 2:05 AM.

Details

Summary

cos function is much much more accurate than cosf.
this patch replaces cosf by cos to gain more accuracy.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
akanad created this revision.Nov 19 2019, 2:05 AM
akanad requested review of this revision.Nov 19 2019, 2:05 AM
jsuya added a comment.Nov 19 2019, 2:50 AM

I agree that change cosf to cos.
But in my opinion, we should to keep note messages. And you also need to change the test. (src/tests/eina/eina_test_matrix.c)

akanad updated this revision to Diff 26979.Nov 19 2019, 3:25 AM
  • keep meaning of removed comment.
  • modify a corresponding testcase.
akanad updated this revision to Diff 26980.Nov 19 2019, 3:42 AM

restore everything, just remove a 'f' characters

vtorri added a subscriber: vtorri.Nov 19 2019, 3:46 AM

is this accuracy really needed ?

Yes. I think it's needed. Because

  1. We've already had a kind of accuracy problem that @jsuya mentioned before D10467
  2. Float is even more inaccurate than we expect. it is not too hard to find that error accumulating when we are using floating pointer in normal usages.
  3. Cost of cos/sin functions are too tiny, so that we don't have to concern about in performance manner in modern processor even embedded
  4. Lastly, you've suggested to use cos rather than cosf before at D10467, jsuya agreed this patch as you can see his comment above. and I agree also.

"Lastly, you've suggested to use cos rather than cosf before at D10467, jsuya agreed this patch as you can see his comment above. and I agree also."

no, i've said that cosf returns a float and i've given a link, that's all

I see, I misunderstood.
I thought that you suggested to use cos rather than cosf because Eina_Matrix{N} has double type structure variables.

@cedric
ping~!
would you please let me know your opinion about this?

jsuya added a comment.Nov 20 2019, 6:45 PM

If there is no opinion on this issue, I will accept it in the afternoon(maybe 4hour later?) :)

jsuya accepted this revision.Nov 21 2019, 12:48 AM
This revision is now accepted and ready to land.Nov 21 2019, 12:48 AM
This revision was automatically updated to reflect the committed changes.