GEM-benchmark / GEM-metrics

Automatic metrics for GEM tasks
https://gem-benchmark.com
MIT License
60 stars 20 forks source link

[WIP] Adding Docker version of Prism #68

Closed danieldeutsch closed 2 years ago

danieldeutsch commented 2 years ago

Here is what it would look like to add a Dockerized version of a metric.

danieldeutsch commented 2 years ago

If you have a machine with Docker installed, you should be able to directly run test_prism.py. The first time it runs, this Docker image will automatically be downloaded to your machine, which may take 1-2 minutes. After that, you can directly run the metric without downloading the Docker image.

danieldeutsch commented 2 years ago

@sebastianGehrmann @tuetschek @ndaheim Please take a look and let me know what you think.

sebastianGehrmann commented 2 years ago

Hey, given the discussions in the chat, should we merge this so we can proceed with adding the other implementations?

danieldeutsch commented 2 years ago

I think it should be ok. My only concern is how to deal with the GPU device. It seems like the other GPU-based metrics don't manually control the device, but this is necessary for the Dockerized metrics or else they will just use GPU 0.

When you run something in Docker, the code run in Docker runs in its own process, so the CUDA_VISIBLE_DEVICES environment variable you might set from the command line is ignored. My library sets that variable specifically for the Docker processes.

I was actually thinking about this last night. Since the classes are instantiated here without any arguments to the constructor, there isn't an obvious way right now to pass the device ID to the Dockerized metrics. https://github.com/GEM-benchmark/GEM-metrics/blob/9435858b3fa211007d75449732264ff62c72f1d9/gem_metrics/__init__.py#L124-L126

Any thoughts?