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

Extend basic operations #174

Merged

Conversation

stewart123579
Copy link
Contributor

Extend basic basic operations (i.e. elementwise ops) in three ways:

  1. Allowing for a number instead of an abstract-tensor type as one of the arguments
    • This means something like (.+ A 10) behaves as it reads - i.e. add 10 to each element of A. An alternative would have been to create a tensor the same type/shape as A and set all elements to 10...and then...add to A
  2. Add an elementwise .max and .min
    • It might just be my use-case, but this means creating a ReLU function is then as simple as (.max 0.0 A)
  3. Add elementwise .exp and .log (natural log)
    • These need a unary-operator function, which will look suspiciously like the existing binary-operator, but with only one argument

Additional thoughts

  1. There are no tests. Happy to write them if requested.
  2. The .max and .min functions suffer from the existing issue that if the type of the arguments differ then we can get a TYPE-ERROR from the SETF. See issue #173. So rather than (.max A 0), it's better to be explicit and write (.max A 0.0).

This is predicated on a new unary-operator function that replicates the
binary-operator, but with a single argument.
... and (NUMBER, TENSOR) arguments

This simplifes the workload if, e.g., adding a single number to a
tensor. No need to create a tensor all of a single value just to apply
the operation.
Copy link
Member

@stylewarning stylewarning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but one small request as commented.

a couple small tests are always appreciated too.

@@ -245,6 +279,51 @@ If TARGET is not specified then a new tensor is created with the same element ty
;; choice, like integers.
(binary-operator #'expt source1 source2 target)))

(define-backend-function .max (source1 source2 &optional target)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use DEFINE-EXTENSIBLE-FUNCTION without specialization here so that one has the option to micro-optimize on tensor type in the future?

@stewart123579
Copy link
Contributor Author

Addressed both comments @stylewarning

I'd read the Developer How-To umpteen times and missed that DEFINE-EXTENSIBLE-FUNCTION could be used without specialization. 🤦

@stylewarning stylewarning merged commit cea23a5 into quil-lang:master Jul 4, 2022
@stylewarning
Copy link
Member

Great, thanks!

@stewart123579 stewart123579 deleted the extend-basic-operations branch July 4, 2022 01:50
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 this pull request may close these issues.

2 participants