search

scikit-hep / iminuit

Jupyter-friendly Python interface for C++ MINUIT2
https://scikit-hep.org/iminuit
Other
285 stars 76 forks source link

Stop re-building Minuit when it didn't change #159

Closed cdeil closed 6 years ago

cdeil commented 9 years ago

I'm working on iminuit/_libiminuit.pyx and run python setup.py build_ext --inplace to re-build it after a change. This also compiles Minuit from scratch every time, which takes ~ 30 seconds.

Build log: https://gist.github.com/cdeil/3f86f87f3d04b4de5e13

Re-building Minuit when it didn't change is un-necessary and having to wait is annoying.

@piti118 Is this something that can / should be improved in setup.py to make the build smarter? Or should I be running some other command that doesn't re-build Minuit every time?

piti118 commented 9 years ago

You can work around it with ccache something like CC=ccache clang++ CXX=ccache clang++

should help.

HDembinski commented 6 years ago

I think to fix this one needs to separately compile Minuit2 as a shared library, and let the _libiminuit.so link to it. This would require some setup.py hacking. I think this is rather an enhancement and not a bug in severity. Can we please remove the bug label?

HDembinski commented 6 years ago

Just for record, I proposed offline to @cdeil to switch the build system to cmake, then it would be easy to compile Minuit2 code as shared library, an link _libiminuit.so to it. This pybind11 example demonstrates, how to write a setup.py that calls cmake to do the actual compiling.

cdeil commented 6 years ago

Just to say: I changed my mind a bit in the past years. I now think the user is king, and if developers have to suffer a little bit, that's OK. Concretely I think introducing a dependency on cmake will be a problem for some fraction of our users, and is not worth the gains in simpler setup.py and faster build (because setuptools always re-builds everything and doesn't do a parallel build).

It might / should also be possible to change the setuptools build to work like "make", i.e. to not re-build Minuit2 unless needed. If it's hard to achieve "make" behaviour with timestamps, maybe just checking the presence of the ".o" files is possible? We will very rarely update Minuit2, and in those cases IMO it's acceptable to require devs to run "make clean" manually to get rid of stale ".o" files.

HDembinski commented 6 years ago

Can't we just make setup.py call make instead of cmake? That would also work.

HDembinski commented 6 years ago

But make is also not available everywhere, right?

cdeil commented 6 years ago

But make is also not available everywhere, right?

I'm not sure if it's available on Windows or not. But even if it is, there's the question if the improvement in build outweighs the risk of getting errors on some platforms where make doesn't behave as expected (assuming the current setuptools is working nicely for all users, because we never got error reports in the past years)

HDembinski commented 6 years ago

I asked on SO for a Python-only solution. Apparently, distutils had a way to do what we want, but it was removed in setuptools. https://stackoverflow.com/questions/49266003/setuptools-build-shared-libary-from-c-code-then-build-cython-wrapper-linked No working solution yet, but it seems possible.

HDembinski commented 6 years ago

I solved this in one of my PRs. Following a suggested solution on SO, I monkey-patched a lazy_compile function into distutils CCompiler, which only compiles a file if the source is newer than the object file.

I am closing this.