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

Draft of image patch extraction pipeline - for feedback #167

Closed
wants to merge 7 commits into from

Conversation

tim-rehnstrom
Copy link

This is a draft containing the patch extraction pipeline. Please let me know if you find any errors or if you see something where I can optimize the code.

@tim-rehnstrom tim-rehnstrom changed the title Draft - for feedback Draft of image patch extraction pipeline - for feedback Jun 3, 2024
Copy link
Member

@bcoltin bcoltin left a comment

Choose a reason for hiding this comment

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

My main suggestions are to make sure it is easy to run everything on other people's computers; and where it makes sense, try to put things in the jupyter notebooks into reusable functions in libraries.

@@ -1,2 +1,3 @@
FROM ghcr.io/nasa/isaac_analyst_notebook
ENV DEBIAN_FRONTEND=dialog
RUN pip install torch==2.2.1 torchvision==0.17.1 torchaudio==2.2.1 --index-url https://download.pytorch.org/whl/cu121
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to commit to these exact versions? What is this url?

Copy link
Member

Choose a reason for hiding this comment

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

the link is from the pytorch install instructions here: https://pytorch.org/

Copy link
Member

Choose a reason for hiding this comment

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

the exact versions might be remaining from when we had to support multiple ubuntu versions

"mounts": [
"source=${localWorkspaceFolder},target=/src/isaac/src,type=bind,consistency=cached",
"source=${localWorkspaceFolder}/../../astrobee/src,target=/src/astrobee/src,type=bind,consistency=cached"
"source=${localWorkspaceFolder}/../../astrobee/src,target=/src/astrobee/src,type=bind,consistency=cached",
"source=${localWorkspaceFolder}/../../data/Sock_example,target=/src/data/vent,type=bind,consistency=cached"
Copy link
Member

Choose a reason for hiding this comment

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

How does this directory get here? Will this cause problems for people who don't have your exact setup or use the analyst notebook for other purposes?

Copy link
Author

Choose a reason for hiding this comment

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

That is left from testing I had done in the past. I will remove it, so no issues occur for others using the notebook

Copy link
Member

Choose a reason for hiding this comment

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

Added by mistake?

Copy link
Author

Choose a reason for hiding this comment

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

Yes it should not be included. It is an old file that slipped through. Files that are deleted within jupyterlab get put in the .trash-0 folder.

"source": [
"from scripts.load_bag_database import LoadBagDatabase\n",
"path=\"data/bags/\"\n",
"path=\"data/bags/2024-03-21_tim/bsharp/Fixed\"\n",
Copy link
Member

Choose a reason for hiding this comment

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

not sure you want to commit the changes to this file. They seem specific to you.

Copy link
Author

Choose a reason for hiding this comment

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

They are specific to me. I will adjust it for the final version.

Copy link
Member

Choose a reason for hiding this comment

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

Fix spelling errors

Copy link
Member

Choose a reason for hiding this comment

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

How do other people get the data to run this?

Copy link
Author

Choose a reason for hiding this comment

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

The bagfiles needs to be uploaded to the ArangoDB first before this can be used to extract the targets. The upload can be done with the 1_import_bagfiles.ipynb notebook.

Copy link
Member

Choose a reason for hiding this comment

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

Make this a function query_image.py? Probably doesn't need a separate module?

Copy link
Member

Choose a reason for hiding this comment

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

Probably better not to have _scripts in filename

Copy link
Author

Choose a reason for hiding this comment

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

That makes sense, an oversight on my part. I will adjust the filenames and the notebooks

Copy link
Member

Choose a reason for hiding this comment

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

What is this? Add some explanation

Copy link
Author

Choose a reason for hiding this comment

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

This is one of the subfeatures. It allows the user to select a target in the image and get the coordinates. It is integrated into the gather_training_data notebook for the actual pipeline. I will add some more explanation and it can act as a tutorial as we talked about during Tuesday's meeting.

Copy link
Member

Choose a reason for hiding this comment

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

what is this? add comments

Copy link
Author

Choose a reason for hiding this comment

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

Also a subfeature. Will add comment so it can act as a tutorial.

@marinagmoreira
Copy link
Member

I recommend that you work from a branch with a relevant name to your feature and not develop. This makes it easier for us to checkout your branch and fix something that comes up too.

To create a branch you can do:
git checkout -b switch_id
to push the branch to your origin:
git push -u origin switch_id
and then re-open a new PR

Also, keep your branch updated with our develop, since we push fixes and updates frequently. Right now, your tests in this PR are failing because you haven't merged in the latest opencv updates

@tim-rehnstrom tim-rehnstrom deleted the branch nasa:develop June 17, 2024 21:28
@tim-rehnstrom tim-rehnstrom deleted the develop branch June 17, 2024 21:28
@tim-rehnstrom tim-rehnstrom restored the develop branch June 17, 2024 21:29
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.

3 participants