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

Plutarch.Extra.List and Plutarch.Extra.Ord #580

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SeungheonOh
Copy link
Collaborator

@SeungheonOh SeungheonOh commented Sep 20, 2022

This PR should allow edits by maintainers now. It seems like I have to use personal fork to allow this due to some permission issue.

@kozross @peter-mlabs

@SeungheonOh SeungheonOh marked this pull request as draft September 20, 2022 21:06
@SeungheonOh
Copy link
Collaborator Author

SeungheonOh commented Sep 20, 2022

Test will be added shortly.

Added.

@SeungheonOh SeungheonOh marked this pull request as ready for review September 20, 2022 23:38
argument returns `PJust`, the value is used in the result.
-}
pmapMaybe ::
forall (ell :: PType -> PType) (b :: PType) (a :: PType) (s :: S).
Copy link
Member

Choose a reason for hiding this comment

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

ell?

Copy link
Contributor

Choose a reason for hiding this comment

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

Like the capital letter of the alphabet. Calling to mind 'List'.

_ -> Nothing

instance PlutusType POrdering where
type PInner POrdering = PInteger
Copy link
Member

Choose a reason for hiding this comment

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

why

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should this use SocttPlut?

Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly. I did the first thing that could possibly work, which seemed to perform OK, and actually allowed some optimizations (which I have measurements for).

Copy link
Member

Choose a reason for hiding this comment

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

This is less efficient to match on and not really cheaper to construct. Scott-encoding is probably better.

--
-- We perform the layer parallelism sequentially; it doesn't affect the
-- semantics, it just makes us sad because it's slow.
psort4 ::
Copy link
Member

Choose a reason for hiding this comment

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

What's up with all this duplication? Just make a psortN

Copy link
Member

Choose a reason for hiding this comment

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

The N would be a Haskell Integer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's heavily optimized by @cjay using some sorting network. You wouldn't get that performance without these.

Copy link
Member

Choose a reason for hiding this comment

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

There is a huge amount of code duplication still. It seems like you can encode the sorting network as a table of numbers since the code is mostly the same. You can err for N more than 4.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you mean a table of numbers on the Haskell level, yeah it could probably be done while generating the same code on the script level, but that seems like a pointless exercise to me, unless you want to work with much larger sorting networks.
If you mean a table on the Plutarch-level, that seems useless if you want to sort arbitrary element types. If you can serialize into bytestrings then yeah, you could work with actual indices. But then you'd have to build the intermediate bytestrings. Maybe it would save significant script size on larger sorting networks, but probably with much higher execution cost.

Copy link
Member

Choose a reason for hiding this comment

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

The former, it's much more succinct than adding a new function every time.

Copy link
Member

Choose a reason for hiding this comment

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

Also, you can have a generic fallback for n > 4.

@L-as
Copy link
Member

L-as commented Oct 28, 2022

Do you still want this?

@SeungheonOh
Copy link
Collaborator Author

Yes It would be nice to have, but not urgent.

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.

None yet

4 participants