Skip to content

Conversation

@esmucler
Copy link
Contributor

@esmucler esmucler commented May 7, 2025

Description

Hi folks. This is the implementation of the methods to compute confidence sets that are robust to weak instruments discussed here. We also have a notebook here, but thought it better to wait and get feedback on this PR before submitting that one.

On top of any feedback you might have, we had two questions to ask you:

  1. Regarding naming of the method. Right now it's "unif_confest", as in, uniformly valid confidence set. This is accurate, but a bit cryptical. Other alternatives include "score confidence set" (since it is obtained by inverting a nonparametric score test), or "robust confidence set" (since it is robust to weak instruments). The latter seems the most user friendly, but also robust is a very loaded word that could mean anything. Any thoughts on this?
  2. Right now, the unif_confset method in the DoubleMLIIVM class just returns a list with tuples. Interpreting each tuple as a (possibly infinity) interval, their union is the confidence set returned by the method. Should we add this somehow to the .summary method of the class? Or create some nicer printing method? Let us know what you think.

Reference to Issues or PRs

This issue.

PR Checklist

Please fill out this PR checklist (see our contributing guidelines for details).

  • The title of the pull request summarizes the changes made.
  • The PR contains a detailed description of all changes and additions.
  • References to related issues or PRs are added.
  • The code passes all (unit) tests.
  • Enhancements or new feature are equipped with unit tests.
  • The changes adhere to the PEP8 standards.

@SvenKlaassen SvenKlaassen linked an issue May 8, 2025 that may be closed by this pull request
@SvenKlaassen SvenKlaassen added the new feature new feature label May 8, 2025
@SvenKlaassen SvenKlaassen self-assigned this May 8, 2025
@SvenKlaassen
Copy link
Member

Hi @esmucler and @david26694 ,

thank you very much. This looks already great.
I guess some small addition to the summary would be nice.
I will check the code in the upcoming days and think more on naming.
Are you fine with me pushing some minor changes to the branch?

Additionally, could you add links to important references to your example notebook?
I will add them to the literature overview of the documentation.

@esmucler
Copy link
Contributor Author

esmucler commented May 8, 2025

Thanks Sven. Yes please go ahead and push any changes you see fit.

I'll add references to the notebook, no problem; we also have an upcoming preprint on the method, but I'll create another PR to add it once we have it on Arxiv.

Best

@esmucler
Copy link
Contributor Author

esmucler commented May 9, 2025

References added to the notebook.

@SvenKlaassen
Copy link
Member

SvenKlaassen commented May 9, 2025

I have added some updates.

  1. I have taken up your name of robust_confset. Even if "robust" is very loaded, we frequently use uniformly for joint confidence intervals and it might be confusing to have the same name. I think users will specifically connect the method to weak IVs, so robust seems a to be good choice. But I am open to other suggestions.
  2. Updated the unit tests. Previously the data was only generated once. Then coverage would not really be a helpful check. Added small checks for the exceptions.
  3. Added a short representation in __str__.
  4. Added special cases to _solve_quadratic_inequality and rely on numpy.polynomial for roots. I think this might make the calculations more "robust", if some strange cases occor or we use the inequality at a different place in the code.

@esmucler
Copy link
Contributor Author

Thanks Sven, looks great. Your solution using np.polynomial is definitely better.

@SvenKlaassen
Copy link
Member

Thanks.
From my side the PR would be ready to be merged (coverage is already checked).
The only minor question would be regarding the notebook example:
The final text does not fit the current results (as the standard confint() method already covers the true effect).

@esmucler
Copy link
Contributor Author

esmucler commented May 14, 2025

You are right, I think I messed up with the seed or something.

PS: Indeed, I had to set another seed. Check now, it should be ok.

@SvenKlaassen
Copy link
Member

I agree. A short simulation study in the notebook would also be great and not too complicated. Coverage results might be more convincing.

@esmucler
Copy link
Contributor Author

Added a small simulation; also set the seed from the random library (on top of np.random), just in case.

@SvenKlaassen
Copy link
Member

Looks great.
I will merge the PR. Maybe you can open a PR for your notebook to merge it into the dev branch of the documentation.

@SvenKlaassen SvenKlaassen merged commit 83dd94f into DoubleML:main May 14, 2025
9 checks passed
@SvenKlaassen SvenKlaassen added this to the Release 0.10.0 milestone May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request]: Methods robust to weak instruments

3 participants