FSCT icon indicating copy to clipboard operation
FSCT copied to clipboard

Bug Fix/User Interface TO DO list.

Open SKrisanski opened this issue 2 years ago • 3 comments

TO DO:

  • Improve error handling for when users set a bad plot_centre. In the situation where a point cloud is georeferenced, the plot centre is [0, 0] and the radius is 0 (aka not cropping the point cloud at all), the segmentation will be useless due to 32 bit floating point truncation issues, but the failure reason is not obvious. Solution: need the code to check that the globally shifted point cloud is near the origin. If this is not true, warn the user, but use an automatically calculated plot centre instead of the user specified plot centre.
  • Add ability to handle batch_size of 1.
  • Unit and integration tests, and scripts to verify correct installation for new users.

Done:

  • Fix requirements file.
  • Improve error handling for poorly segmented datasets. Currently, if no terrain points are labeled due to poor segmentation on an unsuitable dataset, an error results when attempting to make the DTM without any terrain points. Catching this error is an opportunity to inform the user to inspect the segmented.las file, and try another dataset to gain familiarity with the limitations. Timeframe: this is a quick fix, and will probably get done with the requirements fix.

SKrisanski avatar Mar 17 '22 11:03 SKrisanski

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.

ylevental avatar Mar 19 '22 18:03 ylevental

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!

SKrisanski avatar Mar 20 '22 01:03 SKrisanski

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

ylevental avatar Mar 20 '22 13:03 ylevental