[Relay] Add foldr1 #2928
[Relay] Add foldr1 #2928
Conversation
There was a problem hiding this comment.
Mostly LGTM. I would suggest using reduceR or reduceRight instead of foldr1. See elm-community/list-extra#15.
python/tvm/relay/prelude.py
Outdated
| @@ -142,6 +142,29 @@ def define_list_foldr(self): | |||
| self.mod[self.foldr] = Function([f, bv, av], | |||
| Match(av, [nil_case, cons_case]), b, [a, b]) | |||
|
|
|||
| def define_list_foldr1(self): | |||
| """Defines a right-way fold over an un-empty list. | |||
There was a problem hiding this comment.
| """Defines a right-way fold over an un-empty list. | |
| """Defines a right-way fold over a nonempty list. |
|
I'd say foldr1 is better as we already have foldr, and it is more consistent. LGTM. |
|
Languages that use reduce also have fold. I also think foldr1 makes the function feel "second class" even though in many instances it's the right function to use instead of foldr. foldr2 is also often commonly defined, yet has little relation to foldr1. |
|
Additionally, TF already has specialized versions of reduce like |
|
@joshpoll it is indeed second class, and foldr should be used when able, as it err on some input. generally speaking it is better to use total function (see https://wiki.haskell.org/Avoiding_partial_functions). |
Co-Authored-By: lixiaoquan <[email protected]>
|
@joshpoll I've applied your change, do you think it is OK to keep the function name as foldr1? |
@joshpoll Thanks for your suggestion about this issue, could you please review?