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

Convert between list-likes in functions like pmap, pfilter, preverse, ... #597

Open
Renegatto opened this issue Nov 3, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@Renegatto
Copy link
Contributor

Summary

Do conversion between list representations wherever possible for better efficiency (assuming).

Details

The example of inefficiency of current state against suggested one:
Let's assume:

f :: ClosedTerm (A :--> B)
toImplement :: ClosedTerm (PBuiltinList A :--> PList B)

Now:

toImplement = phoistAcyclic $ plam $ \xs -> pconvertLists #$ pmap # f # xs

Suggested:

toImplement = phoistAcyclic $ pmap # f

This way its one less iteration thru the list.

Implementation

Is mostly straightforward and only need changes in type signatures.
For example, for pmap:

pmap ::
 ( PListLike list
 , PListLike list' -- ADDED
 , PElemConstraint list a
 , PElemConstraint list' b -- CHANGED
 ) =>
 Term s (
   (a :--> b) :-->
   list a :-->
   list' b -- CHANGED
 )
@TotallyNotChase
Copy link
Collaborator

TotallyNotChase commented Nov 18, 2022

This has been suggested before: #416

I only have two concerns about this:

  • Type inference. I explain this in a comment in the linked issue; but the gist is that chained list operations become painful with this overloaded impl.
  • Script size. Overloaded functions generate a different AST for each unique instantiation. It's not something that'd increase the size of existing code - but it may encourage bad patterns for script size. It's not a big issue though

Since this is really about optimization, the suggestion of simply using a rewrite rule instead seemed better to me at the time. So I never pursued this issue with a quick PR

@L-as
Copy link
Member

L-as commented Nov 19, 2022

we could probably allow the user to choose in some way

@SeungheonOh SeungheonOh added the enhancement New feature or request label Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants