Repository navigation
Conversation
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
scotts
changed the title
|
LGTM. Let's wait for the CIs to finish. |
| ${GPU_FLAG:-} \ | ||
| --tty \ | ||
| --detach \ | ||
| --shm-size=32gb \ |
There was a problem hiding this comment.
Was this chosen for a specific reason ? I don't know what is the setting in torchdata or codec flows so curious
There was a problem hiding this comment.
Pure copy-pasta from the CUDA files. :) I started trying to refactor this a bit, but that become its own thing, and I decided to cut my losses and get this up as-is. I'm not sure if it's worth refactoring this bit in particular, since we may move to using test-infra in a way that they manage the docker container for us.
| # Note that we are making these CPU-only builds through cmake flags. | ||
| docker exec -t -w "/kineto/build_static" "${container_name}" bash -c " | ||
| set -eux | ||
| cmake -DLIBKINETO_NOCUPTI=1 -DKINETO_LIBRARY_TYPE=static ../libkineto/ |
There was a problem hiding this comment.
We should add other flags as well ? https://github.com/divyanshk/kineto/blob/main/libkineto/CMakeLists.txt#L90
(not a big fan of this - I can consolidate them later)
There was a problem hiding this comment.
In principle, yes, in practice, we get CPU-only builds. I also know those are not available in this docker container. I think we should also verify that we have CPU-only builds, but I'd like that also to be a part of refactoring these later.
There was a problem hiding this comment.
Ya know what, let's just add the flags. :)
There was a problem hiding this comment.
Related: we might want to define some form of CPU_ONLY flag that we can set that inside the build we error if any GPUs were set, and then explicitly set all known GPU flags to 0.
Summary: Depends on pytorch/kineto#1222. Pull Request resolved: pytorch/kineto#1231 Reviewed By: atalman Differential Revision: D91166410 Pulled By: scotts fbshipit-source-id: e237b9c22815d9725bf4aa548085f73098939658 Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
This PR:
linux_cuda.yml.linux_cpu.ymlfor CPU-only build and test.Note that the CPU-only tests run on linux.12xlarge. If we use a smaller system, the PyTorch build will take much longer. Initially, I was using smaller systems while still doing parallel builds and the PyTorch build kept getting killed around 20 minutes in because of OOMs.
I also experimented with not pulling a docker image and trying to use test-infra in a different way. That's still something we should do, but what I was trying was not obviously going to work quickly. We can figure that out later, as I think there's value in getting these CI jobs up as soon as possible.
The CPU-only build and test is also a copy-paste of the existing CUDA tests, with some minor changes. I think that's also fine for now. I think cleaning that up is connected to using test-infra better. Again, I'd like to merge this quickly so we can make sure we have these signals and do this better later (but soon).