Added defs that extract values of a map as a set and as a list #1282
Conversation
examples/spells/commonSpells.qnt
Outdated
| /// - @param __map a map from type a to type b | ||
| /// - @returns the list of all values in the map | ||
| pure def mapValuesList(__map: a -> b): List[b] = { | ||
| __map.keys().fold([], (__list, __k) => __list.append(__map.get(__k))) |
There was a problem hiding this comment.
I have two thoughts here:
Similar to the discussion at #1107 (comment), a snag with operators that take sets to lists is that fold's behavior is undefined for non-commutative operations (like append), due to sets being unordered.
Perhaps it would suffice if we have the toList method from #1107 (fixed as described in the liked comment) together with the mapValusSet added here. Then mapValuesList would be available just as the composition m.mapValuesSet().toList(f) (where f is the ordering function needed to ensure well defined behavior), without needing another named operator.
WDYT?
There was a problem hiding this comment.
I totally agree with Shon here. It would be much better to isolate the potentially problematic behavior of fold in one place
There was a problem hiding this comment.
Nice! In that case, I'll plan to make the needed change to the previous pr, and help see these through.
There was a problem hiding this comment.
I understand the concern, @shonfeder ! However, the problem with using m.mapValuesSet().toList() in place of mapValuesList is that it removes duplicates.
Think about values of the mapping being shares, and we wanted to get the sum of all of them. What would be a preferred way to achieve that? (That was the original use-case, and I perhaps jumped too quickly to a solution.)
There was a problem hiding this comment.
Ah, I see! I didn't think about that :)
We could implement this taking the comparator then. Alternatively, we could provide a foldMap : (acc, (key, value)) => acc => acc if the intermediate representation as a list is not really essential.
There was a problem hiding this comment.
Maybe what would be best here is to have the inverse of setToMap: mapToSet : Map[a, b] -> Set[(a, b)]. Then one could fold over this as needed. We may need some warning about only using this on maps with a finite domain, but I think it would likely be useful. WDYT, @konnov?
There was a problem hiding this comment.
Maybe what would be best here is to have the inverse of
setToMap:mapToSet : Map[a, b] -> Set[(a, b)]. Then one could fold over this as needed. We may need some warning about only using this on maps with a finite domain, but I think it would likely be useful. WDYT, @konnov?
Yes, this makes sense to me. This would simply convert a map to a relation. I thought that we already had something like that in Apalache, but we only have it in the opposite direction.
There was a problem hiding this comment.
I've made the discussed change. @ivan-gavran, do you think this would be suitable for the motivating usecase?
There was a problem hiding this comment.
Absolutely. Thanks :)
Co-authored-by: Gabriela Moreira <[email protected]>
|
Thanks for the contribution and for flagging the need for this, @ivan-gavran! |
This PR adds to spells (added to
commonSpells):mapValuesSet)mapValuesList)CHANGELOG.mdfor any new functionalityREADME.mdupdated for any listed functionality