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

Plot Median AE instead of mean, add mean to metrics #439

Closed
wants to merge 4 commits into from

Conversation

rodvrees
Copy link
Contributor

Fixes #436 according to @mlocardpaulet's suggestion

Important: the changes made here do mean that all the datapoints currently plotted are plotted incorrectly, because metrics for these datapoints cannot be retroactively recalculated (I think)

@mlocardpaulet
Copy link
Contributor

This will shift the points to the left, so we need to re-upload them all. We (I) did this once. I could do it again if needed.

@wolski wolski self-requested a review November 16, 2024 12:24
@wolski
Copy link
Contributor

wolski commented Nov 21, 2024

@mlocardpaulet We must wait with the merge, until we set up a way to recompute all the points?

@mlocardpaulet
Copy link
Contributor

@mlocardpaulet We must wait with the merge, until we set up a way to recompute all the points?

Yes. I think that this is necessary.

@RobbinBouwmeester
Copy link
Contributor

@mlocardpaulet We must wait with the merge, until we set up a way to recompute all the points?

Yes. I think that this is necessary.

Would like to get this up and going ASAP. Should be done before Christmas.

@mlocardpaulet
Copy link
Contributor

mlocardpaulet commented Dec 8, 2024

@mlocardpaulet We must wait with the merge, until we set up a way to recompute all the points?

Yes. I think that this is necessary.

Would like to get this up and going ASAP. Should be done before Christmas.

In the end, the idea is to:

  • have the plot with mean values showing up by default when opening the module page
  • have a switch button that would allow to plot the median for all points (instead of mean).

Now the mean AND median are calculated at each submission, but we do not have the median values for points that were uploaded before now.

There is an issue opened for setting up the automatic re-running of all points already submitted (while keeping their unique ID and associated comments): #470. Since the "main" plot will not change, maybe we do not have to wait for the issue to be fixed. The only thing is that until we are able to re-run everything there will have points missing in the plot with the median?

@rodvrees
Copy link
Contributor Author

rodvrees commented Dec 8, 2024

Mentioned in original issue, but relevant here too:
#436 (comment)

@RobbinBouwmeester
Copy link
Contributor

RobbinBouwmeester commented Dec 9, 2024

@mlocardpaulet We must wait with the merge, until we set up a way to recompute all the points?

Yes. I think that this is necessary.

Would like to get this up and going ASAP. Should be done before Christmas.

In the end, the idea is to:

  • have the plot with mean values showing up by default when opening the module page
  • have a switch button that would allow to plot the median for all points (instead of mean).

Now the mean AND median are calculated at each submission, but we do not have the median values for points that were uploaded before now.

There is an issue opened for setting up the automatic re-running of all points already submitted (while keeping their unique ID and associated comments): #470. Since the "main" plot will not change, maybe we do not have to wait for the issue to be fixed. The only thing is that until we are able to re-run everything there will have points missing in the plot with the median?

Saving of the raw data is implemented. Only thing that remains is a notebook that allows for automatic reuploading. But I think we should do that manually for now. Are you OK with that?

Automatic reupload should be done from:

https://proteobench.cubimed.rub.de/datasets/

As this is currently not supported (most saved submissions are wrong; or incomplete). Only in the latest version everything is correct.

@@ -205,7 +205,7 @@ def plot_metric(
width=700,
height=700,
xaxis=dict(
title="Mean absolute difference between measured and expected log2-transformed fold change",
title="Median absolute difference between measured and expected log2-transformed fold change",
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldnt the title be changed based on what is plotted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and that's what's currently implemented on https://github.com/Proteobench/ProteoBench/tree/mean_median_plot

I'll make a new PR from that branch, then I think this one should be closed without merging

@mlocardpaulet
Copy link
Contributor

@mlocardpaulet We must wait with the merge, until we set up a way to recompute all the points?

Yes. I think that this is necessary.

Would like to get this up and going ASAP. Should be done before Christmas.

In the end, the idea is to:

  • have the plot with mean values showing up by default when opening the module page
  • have a switch button that would allow to plot the median for all points (instead of mean).

Now the mean AND median are calculated at each submission, but we do not have the median values for points that were uploaded before now.
There is an issue opened for setting up the automatic re-running of all points already submitted (while keeping their unique ID and associated comments): #470. Since the "main" plot will not change, maybe we do not have to wait for the issue to be fixed. The only thing is that until we are able to re-run everything there will have points missing in the plot with the median?

Saving of the raw data is implemented. Only thing that remains is a notebook that allows for automatic reuploading. But I think we should do that manually for now. Are you OK with that?

Automatic reupload should be done from:

https://proteobench.cubimed.rub.de/datasets/

As this is currently not supported (most saved submissions are wrong; or incomplete). Only in the latest version everything is correct.

Yes, thanks. Manual re-upload with a Jupiter notebook is great.
If the old submissions cannot be reloaded, we can add them again. I can make this Happen (Holda has all the MQ data, Olivier can re-upload the i2MassChroQ, not sure about alphapept but I can find out).
We'll just need to "clean up" the server and remove all wrong submissions.
Just let me know when we want to do that and if/how I can help.

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.

Datapoint metric is called 'median_abs_epsilon' but is actually mean
4 participants