microsoft / knossos-ksc

Compiler with automatic differentiation
Other
45 stars 10 forks source link

Minimal Python typing with mypy #879

Closed cgravill closed 3 years ago

cgravill commented 3 years ago

AB#19478

Very minimal start, just checks the Torch front-end to KSC. I've moved the timing code out to make the type checking easier to put in - this timing code may actually be redundant now @awf are you keen to keep it still?

cgravill commented 3 years ago

For reasons I'm not clear on, when I run locally mypy src/python/ksc/torch_frontend.py it only checks that file (Windows / Ubuntu-wsl). On CI it checks more files. So I've adjusted it to mypy --follow-imports=skip src/python/ksc/torch_frontend.py so we can start small and focused.

Have you seen anything like this @acl33 ? The difference in behaviour between my local and CI makes me a bit nervous I've something configured wrong somewhere.

cgravill commented 3 years ago

There's also lltm code in there, which it would be good to cf with examples/dl-lltm, and begin to include lltm in our benchmarks.

I've added some links from AB#19585 to this code, and the ADBench lltm that @dcrc2 worked on.

ryotatomioka commented 3 years ago

@cgravill yep my observation is similar. I found it misleading to run mypy on a single file / directory. At least we need to make sure that we are running mypy on the same files (in the same order?) to reproduce CI errors locally.

Some more information here https://mypy.readthedocs.io/en/stable/running_mypy.html

interesting quote:

If you get any of these errors on an import, mypy will assume the type of that module is Any, the dynamic type. This means attempting to access any attribute of the module will automatically succeed:

This is why mypy is more permissive when some files are missing.

cgravill commented 3 years ago

To be explicit, I was running the same mypy src/python/ksc/torch_frontend.py locally and on the CI build and getting different results. It might be down to discovery/accessible paths/extra type stubs I've acquired. I also wondered if the CI machines have some sort of global configuration differences, however, the mypy docs are very explicit that there's no hierarchical merging of config https://mypy.readthedocs.io/en/stable/config_file.html#the-mypy-configuration-file

I like the resolution of doing everything, and getting it to type check properly.