-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
switch-op design #8031
switch-op design #8031
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent
candition2 = pd.and(pd.less_then(100000, global_step), pd.less) | ||
|
||
switch = ScalarSwitchOp() | ||
with switch.case(candition1): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do these three case
similar to if-else
or just if
? a little vague.
if it is like
if cond1: pass
elif cond2: pass
else: pass #cond3
the three switch.case
seems no logic relation with each other.
if it is just three if
like
if cond1: pass
if cond2: pass
if cond3: pass
then they shouldn't share the same Switch
instance.
Need some improvement to specify what logic it is in first glimpse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this is a problem, we have several conditions and the relation is not clear, I will think about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Superjomn
After some search, I think this situation can be more accurately described by cond
op in Lisp https://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node84.html
and the interface can be:
def piecewise_decay():
global_step = pd.Var(shape=[1], value=...)
learning_rate = pd.Var(shape=[1], value=...)
candition1 = pd.less_equal(global_step, 100000)
candition2 = pd.and(pd.less_then(100000, global_step),
pd.less_equal(global_step, 110000))
cand_op = ScalarCandOp()
with cand_op.case(candition1):
pd.assign(learning_rate, 1.0)
with cand_op.case(candition2):
pd.assign(learning_rate, 0.5)
with cand_op.default_case():
pd.assign(learning_rate, 0.1)
return learning_rate
there are some features for cond
op:
-
- the order of condition is useful, the op will find a suitable branch to run from up to down in order.
-
- only one branch will run.
@@ -0,0 +1,75 @@ | |||
### `scalar_switch_case_op` Design |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see.. The point is switch op, but not scalar conditions. Let's change the title to be
# Design Doc: SwitchOp
### `scalar_switch_case_op` Design | ||
|
||
### Background | ||
In a general programing language, there are many scalar control flow operators such as `if` `else` `switch-case`, these operators take scalar bool values as condition input, then decide which block of code the program should run. PaddlePaddle fluid also needs such kind of operators. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let us shorten this paragraph to be:
In many programming languages, there is
switch
, as a generalization form ofif-elif-else
. This design doc is about the implementation ofswitch
for Fluid.
### Background | ||
In a general programing language, there are many scalar control flow operators such as `if` `else` `switch-case`, these operators take scalar bool values as condition input, then decide which block of code the program should run. PaddlePaddle fluid also needs such kind of operators. | ||
|
||
### For example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong English grammar. Let's change it into
### Example 1. Learning Rate Decay
I filed a PR to rewrite this document, to correct English grammar, to make it concise and shorter. Please review it @jacquesqiao . Thanks. |
Update switch design doc
@wangkuiyi thank you for your review and update! |
doc/design/switch.md
Outdated
fluid.print("Case 2") | ||
with cond.default(): | ||
fluid.print("Case 3") | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to scope cond
s to underline that these three conds are in the same scope.
Currently, the cases should be adjacent, but the with
syntax does not have that constraint semantically, so user tends to feel weird and write some wrong code.
what if user write some code like
cond = fluid.switch()
with cond.case(case1):
pass
with cond.case(case2):
pass
b = a + c
with cond.case(cond3):
pass
the b = a + c
in the cases will break the global scope/continuous relations of a switch
-cases.
Discussed with @jacquesqiao
In lisp, a cond
op is like
;; cond has a scope to wrap all the following cases
(cond ((> a 2) "gt")
((= a 2) "eq")
(t "lt")
) ;; cond
it seems better to write code as follows
cond = fluid.switch()
with fluid.switch() as switch:
'''
A limitation inside a `switch`-block that no operators can insert between `case`s.
This rule is easier to check.
'''
with switch.case(fluid.less_equal(a, 10)):
pass
with switch.case(fluid.larger(a, 1)):
pass
switch()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe need a cond.scope() as follow, one cond must and only has one scope.
cond = fluid.switch()
with cond.scope():
'''
A limitation inside a `switch`-block that no operators can insert between `case`s.
This rule is easier to check.
'''
with switch.case(fluid.less_equal(a, 10)):
pass
with switch.case(fluid.larger(a, 1)):
pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fix: #8027