Glow: [wishlist] Migrate bundle building from make to cmake

Created on 8 Nov 2018  路  8Comments  路  Source: pytorch/glow

We have makefiles for the bundle examples, but there's no reason (as far as I can see) we couldn't create real cmake targets for them, which would make the workflow way more accessible. I've automated some slightly different workflows this way, so happy to give advice to anyone who wants to try this.

wishlist

Most helpful comment

I decided to do this since I need learn cmake anyway for one of my other big tasks. I can't figure out how to get add_executable to pick up and link against an object file built using add_custom_command (i.e. the bundle) ~as well as dynamically link against libpng~. Once I have have that, I will be able to post a PR.

cc: cmake master @compnerd

All 8 comments

The intent was to give people a small example that they can copy into their own project. I think that @opti-mix first implemented this in a small shell script.

I decided to do this since I need learn cmake anyway for one of my other big tasks. I can't figure out how to get add_executable to pick up and link against an object file built using add_custom_command (i.e. the bundle) ~as well as dynamically link against libpng~. Once I have have that, I will be able to post a PR.

cc: cmake master @compnerd

Never mind, figured everything out and posted a PR!

@nadavrot I was thinking about your comment on this issue, and I think it might be valuable to have a Makefile example bundle and a CMake example. The CMake stuff in particular is non-trivial, and since CMake is pretty popular I could see people appreciating having an example of how to seamlessly integrate Glow.

@bertmaher @SplitInfinity I see the value of having tests that actually verify that a feature that we care about (AOT compilation) works. In that case, we absolutely need CMake file support.

Probably want to update docs/AOT.md as well.

I'll send another PR to update the AOT docs, but let's also use this issue instead of the PR to discuss what to do with the other bundles and Makefiles.

I think we should add cmake support for all of the bundles (so that everything just "works") and get rid of any that don't add additional value. If I understood @bertmaher 's comments on the PR correctly, the resnet50 bundle is sufficiently recent and complex to serve as a good example of how to make a standalone bundle, and the lenet-mnist bundle is small enough to run in CI. So maybe we should get rid of the other two, and add cmake support for lenet-mnist?

I'm in favor of that approach (convert lenet_mnist to cmake and delete vgg19/zfnet512).

The more I've thought about it the less I like having a split between CMake and Make, and if I had to pick one for an example I'd choose CMake. (My reasoning: getting this working in make is pretty easy, because make is very simple. CMake is harder, so if someone's going to run into a wall with glow it'll be because they can't figure out the CMake part)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

stoklund picture stoklund  路  5Comments

qcolombet picture qcolombet  路  5Comments

pjaaskel picture pjaaskel  路  4Comments

artemrakhov-glow picture artemrakhov-glow  路  4Comments

georgeokelly picture georgeokelly  路  4Comments