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

More restrictive use of the rrule name #26

Closed
yebai opened this issue Oct 23, 2023 · 4 comments
Closed

More restrictive use of the rrule name #26

yebai opened this issue Oct 23, 2023 · 4 comments

Comments

@yebai
Copy link
Contributor

yebai commented Oct 23, 2023

At the moment, we are using rrule in a generic way, in the sense that they can mean both actual rrule for a function and the generated gradient function for a tape. It is better to keep a distinction between these two for better clarity.

@willtebbutt
Copy link
Member

At the moment, we are using rrule in a generic way, in the sense that they can mean both actual rrule for a function and the generated gradient function for a tape. It is better to keep a distinction between these two for better clarity.

I know we discussed this yesterday, but I'm now realising that I'm not entirely sure what you mean. Could you please elaborate?

@yebai
Copy link
Contributor Author

yebai commented Oct 24, 2023

It's about avoiding referring to distribution log density tests as part of rrule tests, and avoiding referring to generated gradient function for unrolled_funciton as rrule.

Technically, this is correct but can cause confusion. It is slightly abusing the rrule terminology I think.

@willtebbutt
Copy link
Member

willtebbutt commented Oct 25, 2023

Ah, right. I agree regarding the location of the distribution tests.

I don't know that I follow your reasoning regarding the unrolled_function rrule!!: it is most certainly an rrule!!, so it ought to be referred to as an rrule!!. It's possible that you find it confusing because the documentation is still being developed, but I don't feel that it's an abuse of terminology. On the contrary, it is a completely correct use of terminology, and IMO it would be confusing to introduce another function purely for the sake of it.

@willtebbutt
Copy link
Member

Resolved by #34

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

No branches or pull requests

2 participants