WIP: add FoldLeft, FoldLeftX; and ForAllX, ForAnyX, FilteredX, NumberX, PerformX #3357
Conversation
fingolfin
added
kind: enhancement
|
I think this looks very reasonable. For the shortcutting, I'm tempted to suggest an optional single argument which is a object which is compared using IsIdenticalObj. My reasoning is:
|
|
@ChrisJefferson I like that idea! We could simply pass |
|
I like this project. |
Codecov Report
@@ Coverage Diff @@
## master #3357 +/- ##
===========================================
+ Coverage 82.18% 93.39% +11.21%
===========================================
Files 678 716 +38
Lines 287772 812207 +524435
===========================================
+ Hits 236500 758590 +522090
- Misses 51272 53617 +2345
|
lib/coll.gd
Outdated
| @@ -2400,6 +2400,38 @@ DeclareOperation( "AsSSortedListNonstored", [IsListOrCollection] ); | |||
| DeclareGlobalFunction( "Elements" ); | |||
|
|
|||
|
|
|||
| ############################################################################# | |||
| ## | |||
| ## TODO: document FoldLeft | |||
There was a problem hiding this comment.
TODO: add documentation
| local tnum, C, func, result, i, l; | ||
| l := Length( arg ); | ||
| if l < 2 or l > 3 or not IsFunction(arg[2]) then | ||
| Error( "usage: FoldLeft( <C>, <func>[, <init>] )" ); |
There was a problem hiding this comment.
TODO: should FoldLeft also get an abort mechanism, like FoldLeftX? How should it work? The one for FoldLeftX is a bit adhoc: it works great for the few applications we have, but it's not very general...
What do other languages do for this, if they do anything at all?
lib/coll.gd
Outdated
| DeclareGlobalFunction( "ForAnyX" ); | ||
| DeclareGlobalFunction( "FilteredX" ); | ||
| DeclareGlobalFunction( "NumberX" ); | ||
| DeclareGlobalFunction( "PerformX" ); |
There was a problem hiding this comment.
TODO: also add FirstX and LastX?
There was a problem hiding this comment.
The "problem" with FirstX is the same as for ForAnyX and ForAllX: we want to be able to short circuit and abort the search as soon as we know the answer. The mechanism for doing this in this PR is the ugliest part (IMHO) of this PR by far (there is an optional abortValue that one can specify, and for which we check by identity comparison... once encountered, the search aborts. Works, is simply, but... ugly?
| while (genIndex <= LEN_PLIST(gens)) { | ||
| Obj gen = ELM_PLIST(gens, genIndex); | ||
| if (IS_FUNC(gen)) | ||
| gen = FastCallFuncList(gen, args); |
There was a problem hiding this comment.
TODO: replace FastCallFuncList by just CallFuncList? It was kind of an attempt to bypass call overhead, but now I wonder if that wasn't a premature optimization. Doing some timing measurements probably would be a good idea: comparing performance with FastCallFuncList versus CallFuncList, but also that of ListX, SumX, ProductX before and after this PR.
fingolfin
force-pushed
the
mh/fold
branch
3 times, most recently
from
711396f
to
edb1c8e
Compare
| UNB_LIST(args, valIndex); | ||
| return 0; | ||
| } | ||
| else if (CALL_1ARGS(IsListOrCollection, gen) == True) { |
There was a problem hiding this comment.
actually here is a chance to generalize ListX and friends to also accept iterators: we just need to accept objects in the filter IsIterator here, that should do it. Note that List and friends already (mostly?) support iterators.
|
|
||
| ############################################################################# | ||
| ## | ||
| #O FoldLeftX( <gens>, <func>, <init>[, <abortValue>] ) |
There was a problem hiding this comment.
TODO: replace abortValue by stopValue or stoppingValue?
| #F FoldLeft( <coll>, <func> ) | ||
| #F FoldLeft( <coll>, <func>, <init> ) | ||
| ## | ||
| InstallGlobalFunction( FoldLeft, |
There was a problem hiding this comment.
Should be redone using InstallEarlyMethod
| Error( "gens[",i+1,"] must be a collection, a list, a boolean, ", | ||
| "or a function" ); | ||
| result:= C[1]; | ||
| for i in [ 2 .. Length( C ) ] do |
There was a problem hiding this comment.
Contrasting this with the code path used when init is given (which uses for i in C do) I think we get different behavior if C has holes. We probably don't want that difference, do we?
This is unfinished work that I've been dragging along in some form or another for a couple years now, but I know some people are interested in similar things, so I thought I should perhaps finally finish it; but then I thought that I'd like to first learn if this has any chance of being merged...
This PR adds a function
FoldLeftand a corresponding methodFoldLeftOp. This generalizesList,Sum,Product.Moreover, it adds kernel function
FOLD_LEFT_X(doesn't need to be a kernel function, but that helps to get competitive performance out of the more general code), and a GAP level wrapper for that calledFoldLeftX. This is then used to reimplementListX,SumX,ProductXwith very little code; and then some "obvious" additional "X"-functions are added:ForAllX,ForAnyX,FilteredX,NumberX,PerformX. Note that these new functions are not just added "for completeness"; I've missed (and then later) used every single one of them at some point.Among the things left to be done:
ForAllXcan immediately abort if it seesfalseI'd be interested to know what people think about the general thrust of this PR, to determine whether there's any point in polishing it up.