Multi-GPU support

[sduxzh]

Hello, Please check the code.

Best wishes Snail

[sduxzh]

Hello, Please check the code.

Best wishes Snail

[borgesa]

Hi,

Nice @sduxzh!

Some comments/questions follow below.

TensorboardX: I believe visualization/writer (TensorboardX) should be separated from 'BaseTrainer'. Maybe in its own module that is imported by BaseTrainer if the configuration file asks for it.

• My reasoning:
  • If the user do not want to use it, then it shouldn't be imported. Also, other visualization frameworks (e.g. Visdom) could be added to the template, so one should not be hard coded into base_trainer
• My proposal:
  • Add separate 'visualization' module to the repository
  • Add class 'WriterTensorflowX' to this module (then we can later add class 'WriterVisdom')
  • In 'BaseTrainer.init' set 'self.writer = self._get_writer(config['writer']) (or some similar configuration parameter
  • In the new function 'BaseTrainer._get_writer' return 'None' if no visualization is set, or return an instance of the selected visualization class using the importlib module.

The above will also enable users to customize their visualization extensively according to their needs (say if they wish to show example output in a GAN model every n'th epoch).

I hope my proposal is clear, if not, I can provide either a proposal or more details on Monday.

Multi-GPU: I have not yet worked much with multi-gpu myself, but I have a couple questions:

• With reference to this tutorial: Shouldn't also the input be distributed to GPU (i.e. 'data_input = data.to(device)')?
• Are there additional requirements to the format of 'config[gpu]'? Should it be a comma separated list with the GPUs to use? If so, I think this should be documented.

[borgesa]

What do we do with this pull request, then?

@SunQpark has separately implemented TensorboardX (in alignment with the proposal I wrote above).

Multi GPU is still not handled.

[SunQpark]

I have handled tensorboardX support with other PR #17 and Multi-GPU implementation in this PR does not cover distributing data into machines. Maybe Multi-GPU support can be done later. I feel sorry for not merging this PR @sduxzh, but I will close this PR.