wichmann-lab / python-psignifit

Python implementation of psignifit, for psychometric function estimation
Other
58 stars 26 forks source link

Bugs in prior plotting - selected sigmoids #151

Open dekuenstle opened 1 month ago

dekuenstle commented 1 month ago

The prior plotting function should add dots for the min, max, quantiles, and point estimate. However, they appear not at the right positions in the plot.

@guillermoaguilar Could you please add the screenshots?

guillermoaguilar commented 1 month ago

MATLAB priors:

Screenshot from 2024-10-07 12-22-25

Python priors: Screenshot from 2024-10-07 12-22-44

Dots positions are different. Black is the point estimate, red and blue the quartiles. Green and yellow are the same, they are the min and max of the range of possible parameter values.

HeikoSchuett commented 1 month ago

The black dot in the prior seems to be still off in #154 . It should be at the prior mean but is clearly not. So the placement is not right yet.

pberkes commented 1 month ago

@HeikoSchuett the black dot is not the mean but the MAP estimate, it looks good from the marginals.

pberkes commented 1 month ago

@HeikoSchuett it's the same in Matlab: https://github.com/wichmann-lab/psignifit/blob/master/plotPrior.m#L97

guillermoaguilar commented 1 month ago

So actually plot_prior still needs correction after I talked to Felix last week (sorry I didnt post it before, I was busy with the docs)

He told me: the points in this plot_priors are supposed to show the range of likely and not likely sigmoids.

BUT if you look at the paper, Fig. 3, it says these are the 0, 25, 50, 75 and 100% quantiles of the prior.

So now I'm confused again.

@HeikoSchuett how should it be?

pberkes commented 1 month ago

@guillermoaguilar "the red and blue should be the 25% and 75% of the sigmoid, not the prior" -> the 25% and 75% of the MAP sigmoid?

pberkes commented 1 month ago

That would work for the threshold plot, but how can we plot the 25% and 75% of the sigmoid in the width or lambda plot?

guillermoaguilar commented 1 month ago

That would work for the threshold plot, but how can we plot the 25% and 75% of the sigmoid in the width or lambda plot?

I don't know. Maybe I understood Felix wrong. Let's see what Heiko answers.

pberkes commented 1 month ago

In any case, as far as I can tell the current code is equivalent to the Matlab version

pberkes commented 1 week ago

@guillermoaguilar @HeikoSchuett PR #154 has been merged... I believe the implementation is equivalent to the Matlab code. Can we close this issue, or are there more changes you'd like to do?