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

Strict maps #102

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

Strict maps #102

wants to merge 2 commits into from

Conversation

treeowl
Copy link
Collaborator

@treeowl treeowl commented Mar 30, 2023

Closes #60

@treeowl treeowl requested a review from konsumlamm March 30, 2023 20:05
src/Data/PQueue/Prio/Internals.hs Outdated Show resolved Hide resolved
src/Data/PQueue/Prio/Internals.hs Outdated Show resolved Hide resolved
src/Data/PQueue/Prio/Internals.hs Outdated Show resolved Hide resolved
@konsumlamm
Copy link
Collaborator

Why no map', traverse', traverseU'?

Also, do we want strict adjustMin etc.? It would be a lot of new functions for something that can easily be done by hand.

Copy link
Collaborator Author

@treeowl treeowl left a comment

Choose a reason for hiding this comment

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

I don't know how many more functions to add. Do we want to do a whole Strict module like containers?

src/Data/PQueue/Prio/Internals.hs Outdated Show resolved Hide resolved
src/Data/PQueue/Prio/Internals.hs Outdated Show resolved Hide resolved
src/Data/PQueue/Prio/Internals.hs Outdated Show resolved Hide resolved
@treeowl
Copy link
Collaborator Author

treeowl commented Mar 31, 2023

One option would be Strict modules that only export the strict functions, and not the rest of the API. That might be less confusing than the way containers does it....

@treeowl
Copy link
Collaborator Author

treeowl commented Apr 1, 2023

Why no map', traverse', traverseU'?

Also, do we want strict adjustMin etc.? It would be a lot of new functions for something that can easily be done by hand.

For adjusting the minimum, we can't do better than

adjustMinWithKey' f = getSolo . adjustMinWithKey ((Solo $!) . f)

Doing it "manually" with the constructors produces exactly the same Core, unfolding, inlining guidance, etc. So arguably we don't need adjustMin' and such, but I wouldn't be opposed to having them for user friendliness.

@treeowl
Copy link
Collaborator Author

treeowl commented Apr 2, 2023

Also, do we want strict adjustMin etc.? It would be a lot of new functions for something that can easily be done by hand.

I just realized I don't understand your last sentence. Implementing adjustMin' using Solo is easy for someone with enough experience, but a lot of people don't find that sort of thing obvious. I don't think it's possible to implement the strict mapping and traversal functions optimally without access to the internals. The only remaining things I can think of are indeed pretty trivial: strict insertion and strict fromList.

@konsumlamm
Copy link
Collaborator

I don't know how many more functions to add. Do we want to do a whole Strict module like containers?

One option would be Strict modules that only export the strict functions, and not the rest of the API. That might be less confusing than the way containers does it....

That might be a good idea, given the amount of strict functions. I agree that such a module should only export the strict functions. I'm also used to the strict version just having a ' suffix though, idk how others would feel about that. Perhaps we should ask for feedback on that (e.g. on the Haskell Discourse or Reddit).

I just realized I don't understand your last sentence. Implementing adjustMin' using Solo is easy for someone with enough experience, but a lot of people don't find that sort of thing obvious. I don't think it's possible to implement the strict mapping and traversal functions optimally without access to the internals.

Yeah, we definitely shouldn't expect everyone to know that you can use Solo here. I meant using minViewWithKey and insert (or the new pattern synonyms), but that wouldn't be O(1).

The only remaining things I can think of are indeed pretty trivial: strict insertion and strict fromList.

For those I'd say let the user force the elements.

@treeowl
Copy link
Collaborator Author

treeowl commented Apr 2, 2023

I'll ask around and see what people think. If we go with a separate module, strict insertion wouldn't be a big deal to include. Even strict extraction could make sense in that context—I wish containers had done that, but it's too late to change now.

minViewWithKey q >>= \((k, v :: Int), q') ->
  Just (... [v + 1])

will produce a thunk to calculate v+1, whereas with

minViewWithKey' q = case minViewWithKey q of
  r@(Just ((_, !_), _)) -> r
  Nothing -> Nothing

this will not.

minViewWithKey' q >>= \((k, v :: Int), q') ->
  Just (... [v + 1])

@konsumlamm
Copy link
Collaborator

I opened a discussion on Discourse: https://discourse.haskell.org/t/organization-of-strict-functions/6354.

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.

Add strict mapping functions
2 participants