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

Refactor of Reduce / Fold operators #38

Closed
wants to merge 12 commits into from

Conversation

quicquid
Copy link

No description provided.

modules/FiniteSetsExt.tla Outdated Show resolved Hide resolved
(* where `x` s the current value of the iterator. *)
(* *)
(* Example: *)
(* FoldSet(LAMBA x,y : x + y, 0 .. 10, 0) = 55 *)
Copy link
Member

Choose a reason for hiding this comment

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

Mismatch: Example mentions FoldSet but operator name is MapThenFoldSet.

Copy link
Author

Choose a reason for hiding this comment

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

Isn't this in the definition of FoldSet by MapThenFoldSet ?

modules/FiniteSetsExt.tla Outdated Show resolved Hide resolved
@quicquid quicquid force-pushed the wip-recursion-operators branch from 08a128e to fc824da Compare March 30, 2021 18:49
modules/FiniteSetsExt.tla Outdated Show resolved Hide resolved
@quicquid quicquid force-pushed the wip-recursion-operators branch 2 times, most recently from f6f0342 to 1dc1c47 Compare March 30, 2021 22:41
Copy link
Contributor

@konnov konnov left a comment

Choose a reason for hiding this comment

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

The order of base and set in calls to FoldSet is inconsistent. I have added comments to fix that.

(* Fold op over the elements of set using base as starting value. *)
(* *)
(* Example: *)
(* FoldSet(LAMBA x,y : x + y, 0 .. 10, 0) = 55 *)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(* FoldSet(LAMBA x,y : x + y, 0 .. 10, 0) = 55 *)
(* FoldSet(LAMBA x,y : x + y, 0, 0 .. 10) = 55 *)

(* An alias for FoldSet. ReduceSet was used instead of FoldSet in *)
(* earlier versions of the community modules. *)
(*************************************************************************)
FoldSet(op, set, acc)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FoldSet(op, set, acc)
FoldSet(op, acc, set)

(* Example: *)
(* Product(1 .. 3) = 6 *)
(*************************************************************************)
FoldSet(LAMBDA x, y: x * y, set, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FoldSet(LAMBDA x, y: x * y, set, 1)
FoldSet(LAMBDA x, y: x * y, 1, set)

(* Example: *)
(* Sum(0 .. 10) = 55 *)
(*************************************************************************)
FoldSet(LAMBDA x, y: x + y, set, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FoldSet(LAMBDA x, y: x + y, set, 0)
FoldSet(LAMBDA x, y: x + y, 0, set)

Comment on lines +42 to +51
FlattenSet(S) ==
(******************************************************************************)
(* Starting from base, apply op to f(x), for all x \in S, in an arbitrary *)
(* order. It is assumed that op is associative and commutative. *)
(* *)
(* Example: *)
(* *)
(* FlattenSet({{1},{2}}) = {1,2} *)
(******************************************************************************)
FoldSet(LAMBDA x,y: x \cup y, {}, S)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is not it exactly UNION S?

(***************************************************************************)
ReduceSet(LAMBDA i, a: op(seq[i], a), DOMAIN seq, acc)
MapThenFoldSet(op, base, LAMBDA i : seq[i], LAMBDA x,y: TRUE, DOMAIN seq)
FoldLeft(op(_, _), base, seq) ==
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FoldLeft(op(_, _), base, seq) ==
FoldSeqLeft(op(_, _), base, seq) ==

(* FoldSeq folds op on all elements of seq from left to right. *)
(***************************************************************************)
MapThenFoldSet(op, base, LAMBDA i : seq[i], LAMBDA x,y: x < y, DOMAIN seq)
FoldRight(op(_, _), base, seq) ==
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FoldRight(op(_, _), base, seq) ==
FoldSeqRight(op(_, _), base, seq) ==

@quicquid quicquid force-pushed the wip-recursion-operators branch from 75790aa to 6a3dd90 Compare April 9, 2021 12:42
Copy link
Contributor

@muenchnerkindl muenchnerkindl left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@lemmy
Copy link
Member

lemmy commented Apr 28, 2021

@quicquid Can you please add a note to https://github.com/tlaplus/CommunityModules/blob/master/README.md#contributing about the line-number business in tests?

@lemmy
Copy link
Member

lemmy commented May 12, 2021

Merged with (squashed) commit 0b5fcd4

@lemmy lemmy closed this May 12, 2021
@lemmy
Copy link
Member

lemmy commented May 13, 2021

@quicquid

(* FoldSeq(LAMBDA x,y: {x} \cup y, {}, <<1,2,1>>) = {1,2} *)
is probably not the greatest example given the existence of { f[x] : x \in DOMAIN f } that is even part of Functions!Range(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants