Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding unary ops (inv, sqrt) #3

Closed
JeffreySarnoff opened this issue Jun 22, 2019 · 6 comments · Fixed by #17
Closed

adding unary ops (inv, sqrt) #3

JeffreySarnoff opened this issue Jun 22, 2019 · 6 comments · Fixed by #17

Comments

@JeffreySarnoff
Copy link
Contributor

I use them quite a bit within arithmetic expressions. If you add them (so I have something to copy), and you want them -- I would PR other widely used unary math functions.

ffevotte added a commit that referenced this issue Jun 22, 2019
@ffevotte
Copy link
Member

Thanks for your comment!

I made the required changes to handle unary ops, in particular sqrt:

julia> @count_ops sqrt(4.3)
Flop Counter:
 add32: 0
 sub32: 0
 mul32: 0
 div32: 0
 add64: 0
 sub64: 0
 mul64: 0
 div64: 0
 sqrt32: 0
 sqrt64: 1

I'm not sure what you meant about inv, though: it seems that inv(x) gets translated early into one(x)/x (number.jl:223), so that the only thing @count_ops sees is a division:

julia> @count_ops inv(4.3)
Flop Counter:
 add32: 0
 sub32: 0
 mul32: 0
 div32: 0
 add64: 0
 sub64: 0
 mul64: 0
 div64: 1
 sqrt32: 0
 sqrt64: 0

If you have ideas for other FP intrinsics which should be counted, please do not hesitate to either add them or tell me. I was thinking of maybe adding comparison operators, FMA and rounding operators, but apart from that I'm short on ideas...

@JeffreySarnoff
Copy link
Contributor Author

JeffreySarnoff commented Jun 22, 2019

fma and muladd are key. both are 3arg functions (so are hypot, clamp).
As I looked at your wonderfully few lines of code, it occured to me that this will work much better for others if there is a very simple way of letting the app know which sets of operators are to be reported. There is no difficulty in counting all that are trackable, my note is just about showing the information.

There are natural sets of them -- I'll put together a look at that.

---- perhaps only instrumenting the ops desired would be more performant

@ffevotte
Copy link
Member

FMA ops are now counted thanks to #12. This also provides an example how to add 3-argument functions.

I do agree that there isn't much difficulty in tracking more instruction types, but showing the information is going to be less and less readable as the number of tracked ops increases. Not sure how to improve on this part though... any idea will be much appreciated!

@JeffreySarnoff
Copy link
Contributor Author

JeffreySarnoff commented Dec 20, 2020

Happy to hear this update. ⛵

julia> @count_ops inv(4.3)
Flop Counter:
  op     32bit   64bit
 add       0       0
 sub       0       0
 mul       0       0
 div       1       0
 sqrt      0       0
 fma       0       0

If all the nonzero entries are for 64bit [32bit] ops, only one column should be shown.
As more ops are included, showing two sets of ops side by side with a little separation is reasonable.

@ffevotte
Copy link
Member

Good idea, thanks!

Filtering empty rows/columns should be relatively easy. Formatting everything in a nicely aligned table should be straightforward too; perhaps even more so if we can leverage the features of PrettyTables.jl. I'll try and see whether the Christmas holidays give me some time to implement something like this.

ffevotte added a commit that referenced this issue Dec 21, 2020
This is a PrettyTables-based implementation of a format similar to what's
suggested in #3
ffevotte added a commit that referenced this issue Dec 21, 2020
This is a PrettyTables-based implementation of a format similar to what's
suggested in #3
@ffevotte
Copy link
Member

The ff/pretty-tables branch gives an overview of how Flop Counter could be displayed as pretty tables. Is that what you had in mind?

I think this is worth the extra dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants