diff --git a/CHANGELOG.md b/CHANGELOG.md index a60ac7a..bab9997 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## 1.5.0 + * Make the `Eq` and `Ord` instances for key-value queues sensible. + Document the nondeterministic nature of these queues. + * Make mapping and traversal functions force the full data structure spine. This should make performance more predictable, and removes the last remaining reasons to use the `seqSpine` functions. As these are no longer diff --git a/src/BinomialQueue/Max.hs b/src/BinomialQueue/Max.hs index ebb2afd..397fcf3 100644 --- a/src/BinomialQueue/Max.hs +++ b/src/BinomialQueue/Max.hs @@ -169,7 +169,7 @@ take :: Ord a => Int -> MaxQueue a -> [a] take n = List.take n . toDescList -- | \(O(k \log n)\)/. 'drop' @k@, applied to a queue @queue@, returns @queue@ with the greatest @k@ elements deleted, --- or an empty queue if @k >= size 'queue'@. +-- or an empty queue if @k >= 'size' queue@. drop :: Ord a => Int -> MaxQueue a -> MaxQueue a drop n (MaxQueue queue) = MaxQueue (MinQ.drop n queue) diff --git a/src/BinomialQueue/Min.hs b/src/BinomialQueue/Min.hs index 3fc0a86..dfd5fa9 100644 --- a/src/BinomialQueue/Min.hs +++ b/src/BinomialQueue/Min.hs @@ -162,7 +162,7 @@ take :: Ord a => Int -> MinQueue a -> [a] take n = List.take n . toAscList -- | \(O(k \log n)\)/. 'drop' @k@, applied to a queue @queue@, returns @queue@ with the smallest @k@ elements deleted, --- or an empty queue if @k >= size 'queue'@. +-- or an empty queue if @k >= 'size' queue@. drop :: Ord a => Int -> MinQueue a -> MinQueue a drop n queue = n `seq` case minView queue of Just (_, queue') diff --git a/src/Data/PQueue/Internals.hs b/src/Data/PQueue/Internals.hs index cd6f0d7..d40e18b 100644 --- a/src/Data/PQueue/Internals.hs +++ b/src/Data/PQueue/Internals.hs @@ -198,8 +198,9 @@ mapEither f (MinQueue _ x ts) Left y -> (fromBare (BQ.insert y l), fromBare r) Right z -> (fromBare l, fromBare (BQ.insert z r)) --- | \(O(n)\). Assumes that the function it is given is monotonic, and applies this function to every element of the priority queue, --- as in 'fmap'. If it is not, the result is undefined. +-- | \(O(n)\). Assumes that the function it is given is monotonic, and applies +-- this function to every element of the priority queue, as in 'fmap'. If it is +-- not, the result is undefined. mapMonotonic :: (a -> b) -> MinQueue a -> MinQueue b mapMonotonic = mapU @@ -274,7 +275,7 @@ insertMinQ' x (MinQueue n x' f) = MinQueue (n + 1) x (BQ.insertMinQ' x' f) -- | @insertMaxQ' x h@ assumes that @x@ compares as greater -- than or equal to every element of @h@. It also assumes, --- and preserves, an extra invariant. See 'insertMax'' for details. +-- and preserves, an extra invariant. See 'BQ.insertMax'' for details. -- tldr: this function can be used safely to build a queue from an -- ascending list/array/whatever, but that's about it. insertMaxQ' :: a -> MinQueue a -> MinQueue a @@ -289,6 +290,9 @@ fromList :: Ord a => [a] -> MinQueue a -- comparison per element. fromList xs = fromBare (BQ.fromList xs) +-- | \(O(n)\). Assumes that the function it is given is monotonic, and applies +-- this function to every element of the priority queue, as in 'fmap'. If it is +-- not, the result is undefined. mapU :: (a -> b) -> MinQueue a -> MinQueue b mapU _ Empty = Empty mapU f (MinQueue n x ts) = MinQueue n (f x) (BQ.mapU f ts) diff --git a/src/Data/PQueue/Min.hs b/src/Data/PQueue/Min.hs index 21733ba..ee2229e 100644 --- a/src/Data/PQueue/Min.hs +++ b/src/Data/PQueue/Min.hs @@ -128,7 +128,7 @@ pattern Empty = Internals.Empty infixr 5 :< -- | A bidirectional pattern synonym for working with the minimum view of a --- 'MinPQueue'. Using @:<@ to construct a queue performs an insertion in +-- 'Prio.MinPQueue'. Using @:<@ to construct a queue performs an insertion in -- \(O(1)\) amortized time. When matching on @a :< q@, forcing @q@ takes -- \(O(\log n)\) time. -- @@ -204,7 +204,7 @@ take :: Ord a => Int -> MinQueue a -> [a] take n = List.take n . toAscList -- | \(O(k \log n)\)/. 'drop' @k@, applied to a queue @queue@, returns @queue@ with the smallest @k@ elements deleted, --- or an empty queue if @k >= size 'queue'@. +-- or an empty queue if @k >= 'size' queue@. drop :: Ord a => Int -> MinQueue a -> MinQueue a drop n queue = n `seq` case minView queue of Just (_, queue') diff --git a/src/Data/PQueue/Prio/Internals.hs b/src/Data/PQueue/Prio/Internals.hs index 38edea7..77ad1a3 100644 --- a/src/Data/PQueue/Prio/Internals.hs +++ b/src/Data/PQueue/Prio/Internals.hs @@ -52,6 +52,7 @@ module Data.PQueue.Prio.Internals ( import Control.Applicative (liftA2, liftA3) import Control.DeepSeq (NFData(rnf), deepseq) +import Data.Function (on) import Data.Functor.Identity (Identity(Identity, runIdentity)) import qualified Data.List as List import Data.PQueue.Internals.Foldable @@ -210,37 +211,18 @@ instance IFoldMap t => IFoldMap (BinomForest t) where foldMapWithKey_ f (Skip ts) = foldMapWithKey_ f ts foldMapWithKey_ f (Cons t ts) = foldMapWithKey_ f t `mappend` foldMapWithKey_ f ts -instance (Ord k, Eq a) => Eq (MinPQueue k a) where - MinPQ n1 k1 a1 ts1 == MinPQ n2 k2 a2 ts2 = - n1 == n2 && eqExtract k1 a1 ts1 k2 a2 ts2 - Empty == Empty = True - _ == _ = False - -eqExtract :: (Ord k, Eq a) => k -> a -> BinomForest rk k a -> k -> a -> BinomForest rk k a -> Bool -eqExtract k10 a10 ts10 k20 a20 ts20 = - k10 == k20 && a10 == a20 && - case (extract ts10, extract ts20) of - (Yes (Extract k1 a1 _ ts1'), Yes (Extract k2 a2 _ ts2')) - -> eqExtract k1 a1 ts1' k2 a2 ts2' - (No, No) -> True - _ -> False +-- | @ (==) = (==) ``on`` 'List.sort' . 'toAscList' @ +instance (Ord k, Ord a) => Eq (MinPQueue k a) where + (==) = (==) `on` toFullySortedList +toFullySortedList :: (Ord k, Ord a) => MinPQueue k a -> [(k, a)] +-- We break up the list to avoid lots of redundant key comparisons +-- in sorting. +toFullySortedList = List.concatMap (List.sortBy (compare `on` snd)) . List.groupBy ((==) `on` fst) . toAscList + +-- | @ compare = compare ``on`` 'List.sort' . 'toAscList' @ instance (Ord k, Ord a) => Ord (MinPQueue k a) where - MinPQ _n1 k10 a10 ts10 `compare` MinPQ _n2 k20 a20 ts20 = - cmpExtract k10 a10 ts10 k20 a20 ts20 - Empty `compare` Empty = EQ - Empty `compare` MinPQ{} = LT - MinPQ{} `compare` Empty = GT - -cmpExtract :: (Ord k, Ord a) => k -> a -> BinomForest rk k a -> k -> a -> BinomForest rk k a -> Ordering -cmpExtract k10 a10 ts10 k20 a20 ts20 = - k10 `compare` k20 <> a10 `compare` a20 <> - case (extract ts10, extract ts20) of - (Yes (Extract k1 a1 _ ts1'), Yes (Extract k2 a2 _ ts2')) - -> cmpExtract k1 a1 ts1' k2 a2 ts2' - (No, Yes{}) -> LT - (Yes{}, No) -> GT - (No, No) -> EQ + compare = compare `on` toFullySortedList -- | \(O(1)\). Returns the empty priority queue. empty :: MinPQueue k a @@ -343,9 +325,11 @@ minViewWithKey (MinPQ n k a ts) = Just ((k, a), extractHeap n ts) mapWithKey :: (k -> a -> b) -> MinPQueue k a -> MinPQueue k b mapWithKey f = runIdentity . traverseWithKeyU (Identity .: f) --- | \(O(n)\). @'mapKeysMonotonic' f q == 'mapKeys' f q@, but only works when @f@ is strictly --- monotonic. /The precondition is not checked./ This function has better performance than --- 'mapKeys'. +-- | \(O(n)\). @'mapKeysMonotonic' f q == 'Data.PQueue.Prio.Min.mapKeys' f q@, +-- but only works when @f@ is strictly monotonic. +-- /The precondition is not checked./ +-- This function has better performance than 'Data.PQueue.Prio.Min.mapKeys'. + mapKeysMonotonic :: (k -> k') -> MinPQueue k a -> MinPQueue k' a mapKeysMonotonic _ Empty = Empty mapKeysMonotonic f (MinPQ n k a ts) = MinPQ n (f k) a (mapKeysMonoF f (const Zero) ts) diff --git a/src/Data/PQueue/Prio/Max.hs b/src/Data/PQueue/Prio/Max.hs index 1bc39d6..66c2545 100644 --- a/src/Data/PQueue/Prio/Max.hs +++ b/src/Data/PQueue/Prio/Max.hs @@ -21,6 +21,9 @@ -- are no guarantees about the relative order in which @k1@, @k2@, and their associated -- elements are returned. (Unlike Data.Map, we allow multiple elements with the -- same key.) +-- The order in which key-value pairs with the same key are extracted, folded, +-- or traversed is explicitly unspecified. From a semantic standpoint, it is +-- simplest to imagine that these operations are nondeterministic. -- -- This implementation offers a number of methods of the form @xxxU@, where @U@ stands for -- unordered. No guarantees whatsoever are made on the execution or traversal order of diff --git a/src/Data/PQueue/Prio/Max/Internals.hs b/src/Data/PQueue/Prio/Max/Internals.hs index 6bfa5ea..3c285f7 100644 --- a/src/Data/PQueue/Prio/Max/Internals.hs +++ b/src/Data/PQueue/Prio/Max/Internals.hs @@ -129,6 +129,8 @@ import Text.Read (Lexeme(Ident), lexP, parens, prec, import Data.Functor.WithIndex import Data.Foldable.WithIndex import Data.Traversable.WithIndex +import qualified Data.List as List +import Data.Function (on) #ifndef __GLASGOW_HASKELL__ build :: ((a -> [a] -> [a]) -> [a] -> [a]) -> [a] @@ -138,11 +140,27 @@ build f = f (:) [] -- | A priority queue where values of type @a@ are annotated with keys of type @k@. -- The queue supports extracting the element with maximum key. newtype MaxPQueue k a = MaxPQ (MinPQueue (Down k) a) -# if __GLASGOW_HASKELL__ - deriving (Eq, Ord, Data) -# else - deriving (Eq, Ord) -# endif +#ifdef __GLASGOW_HASKELL__ + deriving (Data) +#endif + +-- | @ (==) = (==) ``on`` 'List.sort' . 'List.map' 'Data.Ord.Down' . 'toDescList' @ +instance (Ord k, Ord a) => Eq (MaxPQueue k a) where +-- We define an instance rather than deriving one because old Haddock versions +-- choked on documentation in deriving clauses. *sigh*. That's fixed with GHC +-- 8.2. + MaxPQ p == MaxPQ q = p == q + +-- | @ compare = compare ``on`` 'List.sort' . 'List.map' 'Data.Ord.Down' . 'toDescList' @ +instance (Ord k, Ord a) => Ord (MaxPQueue k a) where + compare = compare `on` toFullySortedList + +toFullySortedList :: (Ord k, Ord a) => MaxPQueue k a -> [Down (k, a)] +-- Gosh, this is a mess, but it works. +toFullySortedList (MaxPQ q) = + List.concatMap (List.sortBy (compare `on` Down . snd . unDown)) . + List.groupBy ((==) `on` fst . unDown) . + List.map (\(Down k, v) -> Down (k, v)) . Q.toAscList $ q instance (NFData k, NFData a) => NFData (MaxPQueue k a) where rnf (MaxPQ q) = rnf q diff --git a/src/Data/PQueue/Prio/Min.hs b/src/Data/PQueue/Prio/Min.hs index 2a39a90..4ae2110 100644 --- a/src/Data/PQueue/Prio/Min.hs +++ b/src/Data/PQueue/Prio/Min.hs @@ -26,6 +26,10 @@ -- elements are returned. (Unlike Data.Map, we allow multiple elements with the -- same key.) -- +-- The order in which key-value pairs with the same key are extracted, folded, +-- or traversed is explicitly unspecified. From a semantic standpoint, it is +-- simplest to imagine that these operations are nondeterministic. +-- -- This implementation offers a number of methods of the form @xxxU@, where @U@ stands for -- unordered. No guarantees whatsoever are made on the execution or traversal order of -- these functions. diff --git a/tests/PQueueTests.hs b/tests/PQueueTests.hs index 18807d2..c854fe5 100644 --- a/tests/PQueueTests.hs +++ b/tests/PQueueTests.hs @@ -53,8 +53,8 @@ main = defaultMain $ testGroup "pqueue" , testProperty "foldlU" $ \xs -> Min.foldlU (+) 0 (Min.fromList xs) === sum xs , testProperty "foldlU'" $ \xs -> Min.foldlU' (+) 0 (Min.fromList xs) === sum xs , testProperty "toListU" $ \xs -> List.sort (Min.toListU (Min.fromList xs)) === List.sort xs - , testProperty "==" $ \(xs :: [(Int, ())]) ys -> ((==) `on` Min.fromList) xs ys === ((==) `on` List.sort) xs ys - , testProperty "compare" $ \(xs :: [(Int, ())]) ys -> (compare `on` Min.fromList) xs ys === (compare `on` List.sort) xs ys + , testProperty "==" $ \(xs :: [(Int, Int)]) ys -> ((==) `on` Min.fromList) xs ys === ((==) `on` List.sort) xs ys + , testProperty "compare" $ \(xs :: [(Int, Int)]) ys -> (compare `on` Min.fromList) xs ys === (compare `on` List.sort) xs ys ] , testGroup "Data.PQueue.Max" [ testProperty "size" $ \xs -> Max.size (Max.fromList xs) === length xs @@ -87,8 +87,8 @@ main = defaultMain $ testGroup "pqueue" , testProperty "foldlU" $ \xs -> Max.foldlU (+) 0 (Max.fromList xs) === sum xs , testProperty "foldlU'" $ \xs -> Max.foldlU' (+) 0 (Max.fromList xs) === sum xs , testProperty "toListU" $ \xs -> List.sort (Max.toListU (Max.fromList xs)) === List.sort xs - , testProperty "==" $ \(xs :: [(Int, ())]) ys -> ((==) `on` Max.fromList) xs ys === ((==) `on` List.sort) xs ys - , testProperty "compare" $ \(xs :: [(Int, ())]) ys -> (compare `on` Max.fromList) xs ys === (compare `on` (List.sort . List.map Down)) xs ys + , testProperty "==" $ \(xs :: [(Int, Int)]) ys -> ((==) `on` Max.fromList) xs ys === ((==) `on` List.sort) xs ys + , testProperty "compare" $ \(xs :: [(Int, Int)]) ys -> (compare `on` Max.fromList) xs ys === (compare `on` (List.sort . List.map Down)) xs ys ] , testGroup "Data.PQueue.Prio.Min" [ testProperty "size" $ \xs -> PMin.size (PMin.fromList xs) === length xs @@ -187,7 +187,7 @@ main = defaultMain $ testGroup "pqueue" , testProperty "traverseU" $ \(Fn (f :: () -> Maybe ())) (xs :: [(Int, ())]) -> PMax.traverseU f (PMax.fromList xs) === fmap PMax.fromList (traverse (\(k, x) -> fmap (k,) (f x)) xs) , testProperty "toListU" $ \xs -> List.sort (PMax.toListU (PMax.fromList xs)) === List.sort xs - , testProperty "==" $ \(xs :: [(Int, ())]) ys -> ((==) `on` PMax.fromList) xs ys === ((==) `on` List.sort) xs ys - , testProperty "compare" $ \(xs :: [(Int, ())]) ys -> (compare `on` PMax.fromList) xs ys === (compare `on` (List.sort . List.map Down)) xs ys + , testProperty "==" $ \(xs :: [(Int, Int)]) ys -> ((==) `on` PMax.fromList) xs ys === ((==) `on` List.sort) xs ys + , testProperty "compare" $ \(xs :: [(Int, Int)]) ys -> (compare `on` PMax.fromList) xs ys === (compare `on` (List.sort . List.map Down)) xs ys ] ]