SKrisanski / FSCT

GNU General Public License v3.0
146 stars 60 forks source link

Bug Fix/User Interface TO DO list. #9

Open SKrisanski opened 2 years ago

SKrisanski commented 2 years ago

TO DO:

Done:

ylevental commented 2 years ago

These are all good. Here are my suggestions @SKrisanski :

  1. Correct me if I am wrong, but I still don't see where the sample_dir folders are automatically created in the code. I am still getting the errors of nonexistent folders.
  2. At the bottom of train.py in the parameters, I would prefer device = torch.device('cuda' if torch.cuda.is_available() else 'cpu')
  3. The running_point_cloud_vis = np.vstack... should be changed to running_point_cloud_vis = np.vstack((running_point_cloud_vis, np.hstack((data.pos.cpu() + np.array([i * 7, 0, 0]), data.y.cpu().T, preds.cpu().T))))
  4. I am getting errors such as ValueError: Expected more than 1 value per channel when training, got input size torch.Size([1, 128]) when I run the program after I manually fix 1, 2, and 3. What is the cause of this error? Edit: I set num_procs=18, batch_size=9 and it's working much better. What do you think?

Overall, this is a great program for now, however. Thanks for looking into this.

SKrisanski commented 2 years ago

Thanks for that @ylevental. I very much appreciate the feedback and hearing what challenges users run into. I'll inevitably not catch every bug in development, so it's greatly helpful when people let me know what issues they run into. Also, when working on something for 3 or 4 years, it's easy to forget to mention some of the quirks.

  1. You are correct. Sorry about that, I thought I had put that in, but it turns out that was in my development version still (which has been neglected for a few months due to limited time). I just corrected/added info to that section of the README as a temporary solution. I will improve the handling of this in the near future.

  2. There is a reason I didn't use this one. There are situations (such as when training on my laptop) where I have CUDA, but do not have enough GPU RAM to train a model this large on it. My laptop can fit 2 samples per batch during inference, but cannot handle any training on GPU. I opted to make it an explicit choice for that reason. What I am thinking is to run this check when CUDA mode is selected to make it fall back to CPU if someone selected CUDA mode but CUDA isn't found.

  3. Thanks, I've just fixed that.

  4. This looks to be a batch size issue. If you give it a batch size of 1, it may be losing the batch dimension, causing the size of the tensors to be incompatible. I've added a note to the README to tell users to keep batch size greater than 1 as a temporary solution. I will add nicer handling of this to my TODO list.

Thanks again for the feedback and I hope you find the tool useful!

ylevental commented 2 years ago

You are welcome! I am glad to see your feedback. I am definitely finding FSCT useful for now.