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

Ts mono unifiedmaps #2584

Merged
merged 1 commit into from
Jan 6, 2022
Merged

Ts mono unifiedmaps #2584

merged 1 commit into from
Jan 6, 2022

Conversation

TimSheard
Copy link
Contributor

Introduces UnifiedMap, ViewMap, Triple
Adds SetAlgebra Instances
Adds FromSharedCBOR instances
Fixes files so the new types are actually used

Copy link
Contributor

@JaredCorduan JaredCorduan left a comment

Choose a reason for hiding this comment

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

@TimSheard this really is a great abstraction! It's fantastic how the new code reads so similar to the old! I only had a few trivial comments. I'll wait to approve until my eyes have a change to go over this. Thanks for all this hard work!

Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

This is a pretty big PR.

Here is what I have so far. I'll do a second pass when I am back from vacation.

eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Rules/Ledger.hs Outdated Show resolved Hide resolved
libs/cardano-data/src/Data/Sharing.hs Outdated Show resolved Hide resolved
libs/cardano-data/src/Data/Sharing.hs Outdated Show resolved Hide resolved
libs/cardano-data/src/Data/Sharing.hs Outdated Show resolved Hide resolved
libs/cardano-data/src/Data/UMap.hs Outdated Show resolved Hide resolved
libs/cardano-data/src/Data/UMap.hs Outdated Show resolved Hide resolved
libs/cardano-data/src/Data/UMap.hs Outdated Show resolved Hide resolved
libs/cardano-data/src/Data/UMap.hs Outdated Show resolved Hide resolved
libs/cardano-data/src/Data/UMap.hs Outdated Show resolved Hide resolved
libs/cardano-data/src/Data/UMap.hs Outdated Show resolved Hide resolved
@TimSheard TimSheard force-pushed the ts-mono-unifiedmaps branch from 6b7ed2e to 3aca069 Compare January 3, 2022 20:09
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Few more comments

libs/cardano-data/test/Test/Data/UMap.hs Outdated Show resolved Hide resolved
libs/cardano-data/test/Test/Data/UMap.hs Outdated Show resolved Hide resolved
libs/cardano-data/test/Test/Data/UMap.hs Outdated Show resolved Hide resolved
libs/compact-map/src/Data/Compact/VMap.hs Outdated Show resolved Hide resolved
libs/compact-map/src/Data/Compact/KVVector.hs Outdated Show resolved Hide resolved
@TimSheard TimSheard force-pushed the ts-mono-unifiedmaps branch 4 times, most recently from 5a5c3fc to 972b127 Compare January 5, 2022 16:52
This replaces 3 separate Data.Map in DState, with 1 UnifiedMap.
Added SetAlgebra instances for UMap. Added libs/cardano-data/src/Data/MapExtras.hs
which provide operation on Data.Map not available i the standard library.
Rewrote incrementalStakeDistr to use the Unified reward maps.
Added 50 property tests for UMap.
Added the space saving Trip data and Triple pattern.
@TimSheard TimSheard force-pushed the ts-mono-unifiedmaps branch from 0c7038b to bdf8df5 Compare January 5, 2022 20:08
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Sweet!

@TimSheard TimSheard merged commit 85c75a0 into master Jan 6, 2022
@iohk-bors iohk-bors bot deleted the ts-mono-unifiedmaps branch January 6, 2022 17:09
lehins pushed a commit to lehins/cardano-ledger that referenced this pull request Apr 11, 2022
…ntersectMBO#2584)

This replaces 3 separate Data.Map in DState, with 1 UnifiedMap.
Added SetAlgebra instances for UMap. Added libs/cardano-data/src/Data/MapExtras.hs
which provide operation on Data.Map not available i the standard library.
Rewrote incrementalStakeDistr to use the Unified reward maps.
Added 50 property tests for UMap.
Added the space saving Trip data and Triple pattern.
@lehins lehins mentioned this pull request Jul 5, 2022
3 tasks
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.

3 participants