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

[Feature Request] Enable type checkers #2569

Open
AdrianSosic opened this issue Oct 11, 2024 · 4 comments
Open

[Feature Request] Enable type checkers #2569

AdrianSosic opened this issue Oct 11, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@AdrianSosic
Copy link
Contributor

🚀 Feature Request

BoTorch is pretty much completely typed but currently not usable with type checkers when installed as package into other environments, because there is no py.typed file. This currently requires silencing warnings raised by tools like mypy and thus effectively prevents people from using the available type information when integrating BoTorch into projects. How about activating this feature?

@AdrianSosic AdrianSosic added the enhancement New feature or request label Oct 11, 2024
@Balandat
Copy link
Contributor

We've been contemplating using pyre to do type checking and we'd really like to do that, unfortunately in the past this has caused a lot of headaches due to pyre not working well with pytorch, which resulted in many (thousands!) of spurious errors that we had to manually silence. I haven't looked into this more recently, maybe the situation is better now - if it is or there is an efficient way to deal with this that doesn't require splattering ignores all across the codebase we'd be happy to use type checking in development and the CI.

I guess that is technically a separate question from adding a py.typed file, but given that there currently would likely be a good number of type errors there is a question of the value of doing this.

cc @esantorella, @saitcakmak who probably have additional thoughts on this.

@AdrianSosic
Copy link
Contributor Author

Hi @Balandat, thanks for the quick reply. I know the hassle with pytorch (in fact, we've also globally silenced interaction warnings with it) but I think we need to distinguish between two things here:

  • One is the type checks within botorch, where the problematic interaction with torch occurs as you describe it
  • The other is the use of the existing type information (i.e. purely on a botorch level) by the users of your package

So how about a more pragmatic approach? Instead of fixing pytorch issues, you could simply ignore them entirely (or in whatever reasonable way) but still make your own type hints readable by external tools. Or am I misunderstanding the problem?

@Balandat
Copy link
Contributor

So how about a more pragmatic approach? Instead of fixing pytorch issues, you could simply ignore them entirely (or in whatever reasonable way) but still make your own type hints readable by external tools. Or am I misunderstanding the problem?

No I don't think you're misunderstanding the problem. I guess since we haven't enforced type checking due to the pytorch challenges I know that botorch itself has some typing issues and isn't internally consistent. So I would hesitate to make those erroneous type hints readable by other tools before fixing it. So probably the thing we should do is to (i) silence interaction warnings with pytorch in botorch, (ii) fix any issues with botorch type hints, and (iii) make the type hints readable. This will require a bit of work but it should be worth it.

QQ: how exactly are you silencing the interaction warnings with pytorch in your setup?

@AdrianSosic
Copy link
Contributor Author

I see, thanks for clarification 👍🏼

And sorry for confusion: I just noticed that we only ignored gpytorch imports in our mypy.ini. It seems for pytorch we don't get any warnings by default (actually not sure whether that's because everything is OK or because it's not checked in the first place 🤔 need to investigate 😄 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants