Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alex/open3d ml tutorials #545

Open
wants to merge 5 commits into
base: alex/open3d-ml_tutorials
Choose a base branch
from

Conversation

INF800
Copy link

@INF800 INF800 commented Jun 16, 2022

Re-write condensed tutorials based on suggestions.

Changes were made to 3 tutorial notebooks

  • docs/tutorial/notebook/01_config_files.ipynb
  • docs/tutorial/notebook/02_datasets.ipynb
  • docs/tutorial/notebook/03_train_model.ipynb

Suggested changes were:

  1. The notebooks must look similar to already existing open3d tutorial notebooks (all notebooks)
  2. Run all the notebooks files and check if they work locally (all notebooks)
  3. Working directory must be docs/tutorial/notebook/ for all data, model checkpoint logs etc. (all notebooks)
  4. Remove garbage-in garbage-out dataset graphic. (01_config_files.ipynb)
  5. Use consistent vector notations (01_config_files.ipynb)
  6. Change to right-handed coordinate system (02_datasets.ipynb)
  7. Change colour of axes in the coordinate system (02_datasets.ipynb)

Please note:

  1. Some cells of 02_datasets.ipynb will not run because the notebook does not have access to smaller version of semanticKITTI dataset. @sanskar107 can you share a downloadable link for that dataset that I can wget in the the notebook? Will use shared link https://cdn.discordapp.com/attachments/903644960211992607/946792875423838288/semkitti.zip.
  2. I will pass all the text information through gramarly before merging once structure and content gets green signal.
  3. I will remove Inference_on_a_custom_data.ipynb, reading_a_config_file.ipynb, reading_a_dataset.ipynb, train_ss_model_using_pytorch.ipynb and train_ss_model_using_tensorflow.ipynb before merging because they are essentially duplicates. Keeping them here for reference temporarily.

This change is Reviewable

INF800 added 3 commits June 2, 2022 14:16
+ Condense all notebooks into 3 notebook tutorials.
+ Replace SemanticKITTI with Toronto3D in training notebook (small size & fast training).
+ TODO: Pass through gramarly (may find multiple mistakes), please focus on content & structure.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@INF800 INF800 requested review from sanskar107 and ssheorey June 16, 2022 06:32
@@ -0,0 +1,910 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can access configuration values as object attributes (cfg.{property_name}) or dictionary key values (cfg['{property_name}'])


Reply via ReviewNB

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -0,0 +1,449 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove Open3D-ML for consistency


Reply via ReviewNB

@@ -0,0 +1,449 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standard convention for object detection lidar datasets (e.g. KITTI / Waymo) is x(red) = forward, y (green) = left, z (blue) = up. Add XYZ labels to the axis.

Mention that datasets may instead of r (lidar reflectance) as feature, or no features at all.

A dataset is partitioned into train(ing), valid(ation) and test(ing) splits. Models use data from the training split to update network weights and performance is measured on the validation split. Hyperparameters can be adjusted to optimize performance on the validation split. The test split is never used during the training step and is only used to report the final performance.


Reply via ReviewNB

@@ -0,0 +1,583 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both PyTorch and TF can be installed. User code just needs to choose one inside a specific python file.

requirements-torch.txt if Nvidia GPU is not available.


Reply via ReviewNB

@@ -0,0 +1,583 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section should be in the datasets tutorial.


Reply via ReviewNB

@@ -0,0 +1,583 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

holdout -> test split for consistency


Reply via ReviewNB

@@ -0,0 +1,583 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also talk about:

Continuing training from a checkpoint.

Point to the tensorboard tutorial for visualizing training progress.


Reply via ReviewNB

@@ -0,0 +1,910 @@
{
Copy link
Member

@ssheorey ssheorey Jun 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See Readme.md for how to import Config


Reply via ReviewNB

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -0,0 +1,583 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this listing


Reply via ReviewNB

@@ -0,0 +1,583 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only import the first line and then use the imported namespace.


Reply via ReviewNB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants