Skip to content
This repository has been archived by the owner on Sep 11, 2023. It is now read-only.

_bar_df wrong for loop boundary #1523

Open
MaaikeG opened this issue Dec 16, 2021 · 3 comments
Open

_bar_df wrong for loop boundary #1523

MaaikeG opened this issue Dec 16, 2021 · 3 comments
Labels
Milestone

Comments

@MaaikeG
Copy link

MaaikeG commented Dec 16, 2021

In PyEMMA/pyemma/thermo/extensions/bar/_bar.c

The second for loop should run from 0 to L2, not L1. Current implementation gives an access violation whenever L1 > L2

extern double _bar_df(double *db_IJ, int L1, double *db_JI, int L2, double *scratch)
{
    int i;
    double ln_avg1;
    double ln_avg2; 
    for (i=0; i<L1; i++)
    {
        scratch[i] = db_IJ[i]>0 ? 0 : db_IJ[i];
    }
    ln_avg1 = _logsumexp_sort_kahan_inplace(scratch, L1);
    for (i=0; i<L1; i++)
    {
        scratch[i] = db_JI[i]>0 ? 0 : db_JI[i];
    }
    ln_avg2 = _logsumexp_sort_kahan_inplace(scratch, L2);
    return ln_avg2 - ln_avg1;
}```
@clonker
Copy link
Member

clonker commented Dec 17, 2021

Ouch :) To be fair I think we should just wait until deeptime-ml/deeptime#168 is merged and then back-integrate here with the next deeptime release.

@clonker clonker added this to the PyEMMA 3 milestone Apr 27, 2022
@stale
Copy link

stale bot commented Nov 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 2, 2022
@clonker clonker removed the wontfix label Nov 3, 2022
@stale
Copy link

stale bot commented May 22, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label May 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants