Skip to content

Conversation

@ZiluM
Copy link
Collaborator

@ZiluM ZiluM commented Jul 22, 2025

Hi Feng,

I have updated the independent verification on CFR.
Please check it and merge it if possible.

Zilu

Copy link
Collaborator

@CommonClimate CommonClimate left a comment

Choose a reason for hiding this comment

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

Hi @ZiluM , thank you for implementing the changes.
I suggest a couple of minor modifications:

  • the labels of the figure are wrong. For the lower panels, it should be CE (Coefficient of Efficiency), not Correlation efficiency.
  • independent_verify() (infinitive form) is not very idiomatic. I would prefer indpdt_verif() or indpdt_verification(). Can you change the functions names in both .py modules and update the notebook?
  • lastly, though it wasn't done in Hakim et al 2016, would it make sense to add the numbers of proxies in each class (assimilated/non-assimilated)? Presumably that would add up over all iterations -- I'm hoping that could be done in load_proxylabels()? It would give the audience a sense of how many samples those distributions are based on, which is always important. The other option is to use seaborn.histplot(stat='count') since seaborn is already a package dependency.

@fzhu2e fzhu2e merged commit b969438 into fzhu2e:main Jul 28, 2025
1 check passed
@fzhu2e
Copy link
Owner

fzhu2e commented Jul 28, 2025

@CommonClimate I've addressed the first two bullets in 668748e
please see the updated notebook: https://fzhu2e.github.io/cfr/notebooks/lmr-real-pages2k-indpdt-verif.html

Along with the commit, Python 3.13 is now supported and is the recommended version.

@CommonClimate
Copy link
Collaborator

Thank you so much @fzhu2e . Sorry my email client buries these notifications, so I just caught wind of it. As it happens, I was trying to do the modifications, but I hit a weird pandas bug that I tried to troubleshoot with Zilu, but never could. Anyway, I told @tanaya-g to play with the feature, and I hope she doesn't run into the same bug.
Good news on Python 3.13 as well!

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.

3 participants