Skip to content

Conversation

tbagrel1
Copy link
Member

@tbagrel1 tbagrel1 commented Feb 4, 2022

V n a represents vector having n elements of type a. Such a vector can be built using the make function, taking n linear arguments of type a, and returning a V n a.

At the moment, make uses a lot of type-level machinery around Nats, and is implemented in a way such that it cannot be inlined; even if n is compile-time known.

With #360 (now merged), the Data.V.Linear module has been overhauled, and the elim function has been rewritten to be inlinable.

The aim of this PR is to provide an efficient make implementation using typeclasses so as to make it inlinable when n is known at compile-time. It also adds more symmetry between make and elim, and allows us to get rid of a lot of internal helper functions.

@tbagrel1 tbagrel1 changed the base branch from master to tbagrel1/dupable-new-impl February 4, 2022 10:41
@tbagrel1
Copy link
Member Author

tbagrel1 commented Feb 4, 2022

I set the base of this PR to tbagrel1/dupable-new-impl, so as to reduce clutter in the diff pane, but the idea is to merge this one only after that tbagrel1/dupable-new-impl has been merged into master.

@tbagrel1 tbagrel1 requested a review from aspiwack February 4, 2022 10:50
Base automatically changed from tbagrel1/dupable-new-impl to master February 4, 2022 13:12
@aspiwack
Copy link
Member

aspiwack commented Feb 4, 2022

I know what this PR is about. Today. But please don't make a PR without a description. I'm not the only one who is to read these. And even if this was just for you and me, we may want to revisit the PR in the future to remember why we did X and Y, without an apt description, it becomes impossible.

Copy link
Member

@aspiwack aspiwack left a comment

Choose a reason for hiding this comment

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

Fix the documentation then merge.

We will want to have some tests of the staging of both elim-s as well as make in the test suite before we make a release.

Comment on lines 134 to 150
-- The idea behind 'Make' \/ @make\'@ \/ 'make' is the following:
--
-- @f@ takes @m@ values of type @a@, but returns a @'V' n a@ (with @n@ ≥ @m@),
-- so the @n - m@ missing values must be supplied via the accumulator.
--
-- @make\'@ is initially called with @m = n@ via 'make', and as @m@ decreases,
-- the number of lambda on the left increases and the captured values are put
-- in the accumulator
-- (@V[ ... ] \<\>@ represents the "extend" operation for 'V'):
--
-- > make @n
-- > --> make' @n @n (V[] <>)
-- > --> λx. make' @(n - 1) @n (V[x] <>)
-- > --> λx. λy. make' @(n - 2) @n (V[x, y] <>)
-- > --> ...
-- > --> λx. λy. ... λz. make' @0 @n (V[x, y, ... z] <>) -- make' @0 @n f = f V[]
-- > --> λx. λy. ... λz. V[x, y, ... z]
Copy link
Member

Choose a reason for hiding this comment

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

I think that this documentation should be on the make function, not the Make class.

Plus it speaks of make', which isn't exported. Let's avoid this. If you need to document the implementation, do so outside of the docstring, in regular comments.

Remember to give instantiations of the type of make with concrete values. Examples are very helpful to disentangle very general types.

@tbagrel1 tbagrel1 force-pushed the tbagrel1/v-make-improvement branch from 0b07dc7 to 6e26c50 Compare February 7, 2022 10:33
@tbagrel1 tbagrel1 merged commit 0af41b0 into master Feb 7, 2022
@tbagrel1 tbagrel1 deleted the tbagrel1/v-make-improvement branch February 7, 2022 13:55
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