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

Fixes unit test to generate valid code #892

Merged
merged 1 commit into from
Jul 18, 2022
Merged

Conversation

iomaganaris
Copy link
Contributor

  • Changed g RANGE variable name to g_var to avoid issue with confusing g local variable for current with the instance variable g

C++ code generated

static inline double nrn_current(int id, int pnodecount, Exp2Syn_Instance* inst, double* data, const Datum* indexes, ThreadDatum* thread, NrnThread* nt, double v) {
        double current = 0.0;
        inst->ina[id] = inst->gna[id] * (v - inst->ena[id]);
        inst->g_var[id] = inst->B[id] - inst->A[id];
        inst->i[id] = inst->g_var[id] * (v - inst->e[id]);
        current += inst->i[id];
        current += inst->ina[id];
        return current;
    }


    /** update current */
    void nrn_cur_Exp2Syn(NrnThread* nt, Memb_list* ml, int type) {
        int nodecount = ml->nodecount;
        int pnodecount = ml->_nodecount_padded;
        const int* __restrict__ node_index = ml->nodeindices;
        double* __restrict__ data = ml->data;
        const double* __restrict__ voltage = nt->_actual_v;
        double* __restrict__  vec_rhs = nt->_actual_rhs;
        double* __restrict__  vec_d = nt->_actual_d;
        double* shadow_rhs = nt->_shadow_rhs;
        double* shadow_d = nt->_shadow_d;
        Datum* __restrict__ indexes = ml->pdata;
        ThreadDatum* __restrict__ thread = ml->_thread;
        Exp2Syn_Instance* __restrict__ inst = (Exp2Syn_Instance*) ml->instance;

        int start = 0;
        int end = nodecount;
        #pragma ivdep
        for (int id = start; id < end; id++) {
            int node_id = node_index[id];
            double v = voltage[node_id];
            #if NRN_PRCELLSTATE
            inst->v_unused[id] = v;
            #endif
            inst->ena[id] = inst->ion_ena[indexes[2*pnodecount + id]];
            double g = nrn_current(id, pnodecount, inst, data, indexes, thread, nt, v+0.001);
            double dina = inst->ina[id];
            double rhs = nrn_current(id, pnodecount, inst, data, indexes, thread, nt, v);
            g = (g-rhs)/0.001;
            inst->ion_dinadv[indexes[4*pnodecount + id]] += (dina-inst->ina[id])/0.001*1.e2/inst->node_area[indexes[0*pnodecount + id]];
            inst->ion_ina[indexes[3*pnodecount + id]] += inst->ina[id]*(1.e2/inst->node_area[indexes[0*pnodecount + id]]);
            double mfactor = 1.e2/inst->node_area[indexes[0*pnodecount + id]];
            g = g*mfactor;
            rhs = rhs*mfactor;
            #if NRN_PRCELLSTATE
            inst->g_unused[id] = g;
            #endif
            shadow_rhs[id] = rhs;
            shadow_d[id] = g;
        }
        for (int id = start; id < end; id++) {
            int node_id = node_index[id];
            vec_rhs[node_id] -= shadow_rhs[id];
            vec_d[node_id] += shadow_d[id];
        }
    }

LLVM IR code generated before (llvm branch):

VOID nrn_cur_exp2syn(INSTANCE_STRUCT *mech){
    INTEGER id
    INTEGER node_id, ena_id, node_area_id, ion_dinadv_id, ion_ina_id
    DOUBLE v, g, rhs, v_org, current, dina, mfactor
    for(id = 0; id<mech->node_count; id = id+1) {
        node_id = mech->node_index[id]
        ena_id = mech->ion_ena_index[id]
        node_area_id = mech->node_area_index[id]
        ion_dinadv_id = mech->ion_dinadv_index[id]
        ion_ina_id = mech->ion_ina_index[id]
        v = mech->voltage[node_id]
        mech->ena[id] = mech->ion_ena[ena_id]
        v_org = v
        v = v+0.001
        {
            current = 0
            mech->ina[id] = mech->gna[id]*(v-mech->ena[id])
            mech->g[id] = mech->B[id]-mech->A[id]
            mech->i[id] = mech->g[id]*(v-mech->e[id])
            current = current+mech->i[id]
            current = current+mech->ina[id]
            mech->g[id] = current
        }
        dina = mech->ina[id]
        v = v_org
        {
            current = 0
            mech->ina[id] = mech->gna[id]*(v-mech->ena[id])
            mech->g[id] = mech->B[id]-mech->A[id]
            mech->i[id] = mech->g[id]*(v-mech->e[id])
            current = current+mech->i[id]
            current = current+mech->ina[id]
            rhs = current
        }
        mech->g[id] = (mech->g[id]-rhs)/0.001
        mech->ion_dinadv[ion_dinadv_id] = mech->ion_dinadv[ion_dinadv_id]+(dina-mech->ina[id])/0.001*1.e2/mech->node_area[node_area_id]
        mech->ion_ina[ion_ina_id] += mech->ina[id]*(1.e2/mech->node_area[node_area_id])
        mfactor = 1.e2/mech->node_area[node_area_id]
        mech->g[id] = mech->g[id]*mfactor
        rhs = rhs*mfactor
        mech->vec_rhs[node_id] = mech->vec_rhs[node_id]-rhs
        mech->vec_d[node_id] = mech->vec_d[node_id]+mech->g[id]
    }
}

Code generated after this fix:

VOID nrn_cur_exp2syn(INSTANCE_STRUCT *mech){
    INTEGER id
    INTEGER node_id, ena_id, node_area_id, ion_dinadv_id, ion_ina_id
    DOUBLE v, g, rhs, v_org, current, dina, mfactor
    for(id = 0; id<mech->node_count; id = id+1) {
        node_id = mech->node_index[id]
        ena_id = mech->ion_ena_index[id]
        node_area_id = mech->node_area_index[id]
        ion_dinadv_id = mech->ion_dinadv_index[id]
        ion_ina_id = mech->ion_ina_index[id]
        v = mech->voltage[node_id]
        mech->ena[id] = mech->ion_ena[ena_id]
        v_org = v
        v = v+0.001
        {
            current = 0
            mech->ina[id] = mech->gna[id]*(v-mech->ena[id])
            mech->g_var[id] = mech->B[id]-mech->A[id]
            mech->i[id] = mech->g_var[id]*(v-mech->e[id])
            current = current+mech->i[id]
            current = current+mech->ina[id]
            g = current
        }
        dina = mech->ina[id]
        v = v_org
        {
            current = 0
            mech->ina[id] = mech->gna[id]*(v-mech->ena[id])
            mech->g_var[id] = mech->B[id]-mech->A[id]
            mech->i[id] = mech->g_var[id]*(v-mech->e[id])
            current = current+mech->i[id]
            current = current+mech->ina[id]
            rhs = current
        }
        g = (g-rhs)/0.001
        mech->ion_dinadv[ion_dinadv_id] = mech->ion_dinadv[ion_dinadv_id]+(dina-mech->ina[id])/0.001*1.e2/mech->node_area[node_area_id]
        mech->ion_ina[ion_ina_id] += mech->ina[id]*(1.e2/mech->node_area[node_area_id])
        mfactor = 1.e2/mech->node_area[node_area_id]
        g = g*mfactor
        rhs = rhs*mfactor
        mech->vec_rhs[node_id] = mech->vec_rhs[node_id]-rhs
        mech->vec_d[node_id] = mech->vec_d[node_id]+g
    }
}

In the C++ and fixed LLVM IR code g local variable is used to assign current to while before inst->g[id] was used in LLVM IR that generated wrong code.

Related to #869

@iomaganaris iomaganaris requested a review from pramodk July 8, 2022 12:03
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #64821 (:white_check_mark:) have been uploaded here!

Status and direct links:

Copy link
Contributor

@pramodk pramodk left a comment

Choose a reason for hiding this comment

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

LGTM

…g local variable for current with the instance variable g
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #65850 (:white_check_mark:) have been uploaded here!

Status and direct links:

@iomaganaris iomaganaris merged commit 052e038 into llvm Jul 18, 2022
@iomaganaris iomaganaris deleted the magkanar/fix_exp2syn_test branch July 18, 2022 17:01
iomaganaris added a commit that referenced this pull request Sep 15, 2022
- Changed g RANGE variable name to g_var to avoid issue with confusing g local variable for current with the instance variable g
- Corrected expected code
iomaganaris added a commit that referenced this pull request Sep 15, 2022
- Changed g RANGE variable name to g_var to avoid issue with confusing g local variable for current with the instance variable g
- Corrected expected code
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 this pull request may close these issues.

3 participants