choderalab / fah-xchem

Tools and infrastructure for automated compound discovery using Folding@home
MIT License
6 stars 3 forks source link

Addition of a table plot to assess complex convergence #54

Closed glass-w closed 4 years ago

glass-w commented 4 years ago

Addresses #53

This attempts to implement plotting of a table to aid in analysing RUNs with poor convergence in the complex phase.

Possible TODOs:

glass-w commented 4 years ago

@mcwitt this is probably not as neat as I would like it to be but I've tried to follow the general templates in covid_moonshot/analysis/plot.py

glass-w commented 4 years ago

typo in last commit. Created save_table function.

mcwitt commented 4 years ago

Looks good! I think it's ready to merge when save_table is working the way you want.

glass-w commented 4 years ago

This is now pretty much there. I had to remove the title from the table as it kept being placed in the centre. This table will mainly be used to diagnose runs where complex convergence is not good so I don't think aesthetics matter too much here.

Anyway, the usage has now slightly changed (see this gist).

Here is a small snapshot of what the final PDF looks like (it's all on one PDF page):

Screenshot 2020-09-03 at 14 59 38
mcwitt commented 4 years ago

@WG150 are you envisioning adding this to the sprint status page too (or just accessing via URL?)

hannahbrucemacdonald commented 4 years ago

Would it be possible to float the jobs with the worst stdevs to the top of the list?

Also, could we use RUNX rather than JOBID? They should be the same but it's just more consistent notation!

If these go on the sprint page, it should go alongside the convergence plots that they're associated with!

glass-w commented 4 years ago

@mcwitt I was thinking of just having this accessed via URL rather than on display (since it is only showing RUNs with bad convergence).

@hannahbrucemacdonald yes that's a good idea, I'll sort the worst standard devs to appear at the top of the list. With RUNX instead of JOBID, that'll be an easy fix and I agree that'll be more consistent.

I don't think we should add this next to each convergence plot mainly because 1) the PDF produced is just one long PDF file and 2) We could just add the standard deviation to the convergence plots individually (i.e. as an inset?).

glass-w commented 4 years ago

@mcwitt I've made these changes now, if you're happy with them feel free to merge.