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

90 average calibration #107

Merged
merged 90 commits into from
Sep 3, 2024
Merged

90 average calibration #107

merged 90 commits into from
Sep 3, 2024

Conversation

pasq-cat
Copy link
Contributor

@pasq-cat pasq-cat commented Jul 23, 2024

This pull request add the following things:

  • Add a method to compute the average level of calibration for both classification and regression tasks, using the method proposed by kuleshov in his paper https://arxiv.org/pdf/1807.00263
  • add unit tests and docstrings for the functions implemented.
  • update and expand the documentation on the website.

pasq-cat and others added 30 commits June 9, 2024 05:49
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Member

@pat-alt pat-alt left a comment

Choose a reason for hiding this comment

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

Just a few minor things (see individual comments)

Copy link
Member

Choose a reason for hiding this comment

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

Is this change necessary? I think the existing approach should use the latest release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uhm i thought i removed julia version 1.0 and left 1.10. how do i fix this? replace 1.9 with 1.10?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, nvm, let's keep as is now

Copy link
Member

Choose a reason for hiding this comment

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

I think this qmd needs to be re-rendered one more time?

Copy link
Contributor Author

@pasq-cat pasq-cat Aug 26, 2024

Choose a reason for hiding this comment

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

yes they are not rendered yet because i am still working on them. anyway, now that we are at it, i have a question. whenever i type quarto render tutorials/etc i end up with a lot of files that they are not used. in particular i get 2 types of markdown files and other stuff that i have to delete manually. is there a way to avoid this issue?

Copy link
Member

Choose a reason for hiding this comment

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

As discussed, add this to _quarto.yml

render:
    - "!docs/src/*.md"

end

@doc raw"""
empirical_frequency_classification(y_binary, distributions::Distributions.Bernoulli)
Copy link
Member

Choose a reason for hiding this comment

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

Are these docstrings rendering correctly in the REPL when you use help mode?

Copy link
Contributor Author

@pasq-cat pasq-cat Aug 26, 2024

Choose a reason for hiding this comment

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

i will check one by one later. at the moment i am working on the multi.qmd file. i am failing to make to join the plots in a 2,2 grid. I will upload as it is now , so that you can see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw i was considering deleting the original functions for sampled distributions and leaving only those for normal distributions since laplaceredux only works for this kind of distr. i will submit the general ones to the guys at calibration.jl @pat-alt

test/calibration.jl Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Actually, nvm, let's keep as is now

Copy link
Member

Choose a reason for hiding this comment

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

As discussed, add this to _quarto.yml

render:
    - "!docs/src/*.md"

_quarto.yml Outdated
@@ -2,6 +2,9 @@ project:
title: "LaplaceRedux.jl"
execute-dir: project

render:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pat-alt i tried placing this like piece of code both here and in /docs/_metadata.yml but it doesn't work. if i run quarto render . from inside the /tutorials folder i get 44 new files.

Copy link
Member

Choose a reason for hiding this comment

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

I think this needs to be indented to fall under the project settings:

project:
  title: "{{{PKG}}}.jl"
  execute-dir: project
  render:
    - "!docs/src/*.md"

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by "it stops working completely"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nvm found the solution

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pat-alt the documentation says that when you use the prefix ! "you need to start by specifying everything you do want to render. For example: project:
render:
- "*.qmd"
- "!ignored.qmd"
- "!ignored-dir/"

Copy link
Member

Choose a reason for hiding this comment

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

You're right, thanks!

Copy link
Contributor Author

@pasq-cat pasq-cat Aug 27, 2024

Choose a reason for hiding this comment

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

@pat-alt is there a way to preview the website? i would like to check if everything is okay but there doesn't seem to be a command. Seems like i am missing a step... once i render the .qmd files, how do i rerender the html part?

Copy link
Member

Choose a reason for hiding this comment

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

This is handled by Documenter.jl, simply run julia --project=docs docs/make.jl in the command line. This will render to the docs/build directory. In there, simply click on the index.html file, which will open the (local version of the) website in your browser

@pasq-cat pasq-cat requested a review from pat-alt August 26, 2024 17:55
@pat-alt
Copy link
Member

pat-alt commented Aug 29, 2024

Let me know when you're happy for this to be merged, then I'll review one more time @pasq-cat

@pasq-cat
Copy link
Contributor Author

pasq-cat commented Aug 30, 2024

Let me know when you're happy for this to be merged, then I'll review one more time @pasq-cat

try this version pls. i had to add seed to the arguments of the functions in data.jl. see if it's ok for you. before merging tag me because the guidelines says that the pull request must have a good description of the work done, so i have to edit title and description

Copy link
Member

@pat-alt pat-alt left a comment

Choose a reason for hiding this comment

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

Nice work!

Small comments mostly about typos/grammar.

One thing I noticed is the Calibration_plot function name. Can we just make this calibration_plot please? I guess that's in TaijaPlotting though

docs/src/tutorials/calibration.qmd Outdated Show resolved Hide resolved
docs/src/tutorials/calibration.qmd Outdated Show resolved Hide resolved
docs/src/tutorials/calibration.qmd Outdated Show resolved Hide resolved
docs/src/tutorials/calibration.qmd Outdated Show resolved Hide resolved
docs/src/tutorials/calibration.qmd Outdated Show resolved Hide resolved
docs/src/tutorials/logit.qmd Outdated Show resolved Hide resolved
docs/src/tutorials/multi.qmd Outdated Show resolved Hide resolved
docs/src/tutorials/multi.qmd Outdated Show resolved Hide resolved
docs/src/tutorials/multi.qmd Outdated Show resolved Hide resolved
docs/src/tutorials/multi.qmd Outdated Show resolved Hide resolved
@pat-alt pat-alt merged commit f7bf0f4 into main Sep 3, 2024
9 checks passed
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.

Add a function to compute the average calibration in utils.jl
2 participants