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

Roc curves #49

Merged
merged 15 commits into from
Jan 4, 2023
Merged

Roc curves #49

merged 15 commits into from
Jan 4, 2023

Conversation

melmasri
Copy link
Collaborator

Hi Felix,

I updated the master branch to include

  1. ROC plots. created a different folder for them in the benchmark director
  2. fixed burning to prob for traj plots and autocorrelation.

One thing to look it is the error bard for the roc plots, I think your error bars consider FP/number of edges, while it should be (i think) FP/number of 0's.

Thanks

@melmasri
Copy link
Collaborator Author

Also the doc's in each folder do we change them manually. I need to update the brun in setup in this case.

@felixleopoldo
Copy link
Owner

  1. Great. Once this is in place for MCMC modules, we could also have a dedicated module to produce ROC curves from the edge posterior probabilities.
  2. Thanks. Yes, the docs has to be updated manually. Preferably the field description should be part of the JSON schema but in the meantime it is hard coded in docs.rst. The modules structure is described here and this part describes how to update the docs.

We use FP/P intentionally but FP/#0s is also interesting. Using TP/P and FP/P the SHD/P can be seen in the ROC-like plots as the Manhattan distance to the point (0,1). Have a look at page 31 in the Benchpress paper.

@melmasri melmasri mentioned this pull request Jan 2, 2023
@melmasri
Copy link
Collaborator Author

melmasri commented Jan 3, 2023

🔨

@felixleopoldo felixleopoldo merged commit 939d3ef into master Jan 4, 2023
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.

2 participants