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

Fix nx related work on chokecherry #37

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open

Fix nx related work on chokecherry #37

wants to merge 33 commits into from

Conversation

harryzhang1018
Copy link
Contributor

Don't merge into master branch

Copy link
Collaborator

@AaronYoung5 AaronYoung5 left a comment

Choose a reason for hiding this comment

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

@harryzhang1018 let's clean this up sooner rather than later

atk.yml Outdated
@@ -82,7 +86,7 @@ services:
USER_UID: "@{uid}"
USER_GID: "@{gid}"
APT_DEPENDENCIES: "bash zsh vim git git-lfs python3-pip python3-tk python3-opencv rapidjson-dev"
PIP_REQUIREMENTS: "wa_simulator pandas matplotlib numpy>=1.19 opencv-python tornado black Pillow torch torchvision pyserial nvidia-pyindex"
PIP_REQUIREMENTS: "wa_simulator pandas matplotlib numpy>=1.19 opencv-python tornado black Pillow torch torchvision pyserial nvidia-pyindex casadi cvxpy scipy osqp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we still need these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, "casadi cvxpy scipy osqp" is for MPC, I also need to add pip and apt dependencies for RTK/GPS driver to run

@@ -281,6 +281,10 @@ def LabelConeAssets():
vehicle.SetTireType(tire_model)
vehicle.SetInitPosition(chrono.ChCoordsysD(initLoc, initRot))
vehicle.SetTireStepSize(tire_step_size)
vehicle.SetMaxMotorVoltageRatio(0.16)
Copy link
Collaborator

Choose a reason for hiding this comment

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

needed? is this only in chrono-internal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this needed to be merged into main cuz we are not gonna run simulation on Jetson right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@harryzhang1018 your job is to figure out what we need here (if anything)

Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need all these?

Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty much all of the keras files can be deleted... We have a seperate repo (which should end up somewhere on SBEL github) for training, and it takes <20 min to train each model.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might just be nice to have at least one either on the github or loaded (similar to Yolo)... but that should come from a control node PR not from here




def mpc_cvxpy_solver_v2(xref,yref,v_current,u_current):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep? if yes, please add docstrings

Copy link
Collaborator

Choose a reason for hiding this comment

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

in main, diff

Copy link
Collaborator

Choose a reason for hiding this comment

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

diff with main

Copy link
Collaborator

Choose a reason for hiding this comment

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

can we just create a data folder somewhere that has all these paths?

Copy link
Contributor

Choose a reason for hiding this comment

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

This also doesn't need to be merged, part of Path Planning PR

Copy link
Collaborator

Choose a reason for hiding this comment

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

diff with main


self.go = False
self.error_state = VehicleState()
self.file = open("/home/art-ishaan/art-ishaan/workspace/src/path_planning/path_planning/lot17_sinsquare_sv.csv")
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's not use hardcoded paths

@AaronYoung5
Copy link
Collaborator

i'd recommend cloning locally and deleting stuff you don't need and then pushing back to this branch. Do we need all these datafiles? Can we have a datafolder on box or something that houses all of them?

@harryzhang1018
Copy link
Contributor Author

@AaronYoung5 @StefanCaldararu
About the model, let me know what u guys think about. I have already backup the NN we used for outdoor tracking in the repo here: https://github.com/harryzhang1018/nn_models_lib, and I added you in that repo too

Copy link
Contributor

@StefanCaldararu StefanCaldararu left a comment

Choose a reason for hiding this comment

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

@harryzhang1018 Looked through this. IMO, best way to go about things is as follows:

  • start by making sure everything that we want to keep is backed up. From what I can tell, these are path files and keras files. (I think you've already done this). I think it's fine to have these backed up externally for now, until we find a home for them (we should really take care of Imitation Learning ML training needs to be added somewhere #39).
  • Test the real vehicle with the new changes Aaron has made, and try and get that working.
  • Delete this branch/pr once nothing is needed anymore.

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