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

Consider a redesign of the for loop's body in reverse pass #832

Open
kchristin22 opened this issue Mar 20, 2024 · 0 comments · May be fixed by #835
Open

Consider a redesign of the for loop's body in reverse pass #832

kchristin22 opened this issue Mar 20, 2024 · 0 comments · May be fixed by #835

Comments

@kchristin22
Copy link
Collaborator

kchristin22 commented Mar 20, 2024

The current implementation of the loop's body in the reverse pass of the Reverse mode is not very clear. Consider this example:

double fn(double u, double v) {
    double res = 11 * u;
    for (int i = 0; i < 3; i++) {
        if (i==2){
            res += u * i;
            continue;
        }
        else {
            res -= u * i;
        }
        res += u;
    }
    return res;
}

The for loop in the reverse pass, after the fix of #710, is derived like so:

for (; _t0; _t0--) {
        i--;
        switch (clad::pop(_t4)) {
          case 2UL:
            ;
            {
                res = clad::pop(_t6);
                double _r_d2 = _d_res;
                * _d_u += _r_d2;
            }
            if (clad::pop(_t2)) {
              case 1UL:
                ;
                {
                    res = clad::pop(_t3);
                    double _r_d0 = _d_res;
                    * _d_u += _r_d0 * i;
                    _d_i += u * _r_d0;
                }
            } else {
                {
                    res = clad::pop(_t5);
                    double _r_d1 = _d_res;
                    * _d_u += -_r_d1 * i;
                    _d_i += u * -_r_d1;
                }
            }
        }

Even though the results are correct and the cases handled as expected, the indexing of the code is not very traditional.

The proposed change evaluates this snippet as:

for (; _t0; _t0--) {
        i--;
        switch (clad::pop(_t4)) {
          case 2UL:
            {
                res = clad::pop(_t6);
                double _r_d2 = _d_res;
                * _d_u += _r_d2;
                if (clad::pop(_t2))
                    ;
                else {
                    {
                        res = clad::pop(_t5);
                        double _r_d1 = _d_res;
                        * _d_u += -_r_d1 * i;
                        _d_i += u * -_r_d1;
                    }
                }
                break;
            }
          case 1UL:
            {
                res = clad::pop(_t3);
                double _r_d0 = _d_res;
                * _d_u += _r_d0 * i;
                _d_i += u * _r_d0;
                break;
            }
        }
@kchristin22 kchristin22 linked a pull request Mar 20, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant