uwsbel / autonomy-research-testbed

https://projects.sbel.org/autonomy-toolkit
MIT License
15 stars 5 forks source link

Vehicle Setups #79

Open AaronYoung5 opened 10 months ago

AaronYoung5 commented 10 months ago

1) This assumes that the OS and autonomy-research-testbed is on the same SSD. This is not the case for us (which is a conscious choice) - I don't think we (ART-Broly) would like to change that. 2) We have already done the steps that are outlined in the "setup.sh" script. They are slightly different, but I do not see why each vehicle should be set up exactly the same way, specially for stuff like where the art repo is etc.

Originally posted by @Huzaifg in https://github.com/uwsbel/autonomy-research-testbed/issues/60#issuecomment-1805873467

AaronYoung5 commented 10 months ago
  1. This assumes that the OS and autonomy-research-testbed is on the same SSD. This is not the case for us (which is a conscious choice) - I don't think we (ART-Broly) would like to change that.
  2. We have already done the steps that are outlined in the "setup.sh" script. They are slightly different, but I do not see why each vehicle should be set up exactly the same way, specially for stuff like where the art repo is etc.
  1. I don't believe that is true (that is that this assumes it's on the same SSD). How would that be different? But also had this discussion with @deepak-rlv. It's not clear to me why we don't just use the nvme SSD as the boot device. The only reason NVIDIA includes the emmc SSD is because it's much cheaper, but in all other ways, it's worse.
  2. From a development perspective, you/we are not the only users of each vehicle. For instance, if Ishaan is working with Harry on a project with his vehicle (which he has), if everything is identical (in terms of setup), that is ideal/what we want. Plus from a maintenance perspective, it's unclear to me what the advantage of having different setups on each vehicle is.

@Huzaifg thoughts?

Originally posted by @AaronYoung5 in https://github.com/uwsbel/autonomy-research-testbed/pull/60#issuecomment-1805879479

AaronYoung5 commented 10 months ago

@Huzaifg please use this issue for this discussion instead. Thanks!!

Huzaifg commented 10 months ago

1) Its easier for to access data if the OS gets corrupted and is on a separate SSD (or) if I want to change OS (not sure if it applies here), I can do it without worrying about my data being lost. I like having the comfort of having my data in a place that is separate from the OS because the OS is subject to change. The setup scripts change because all the paths are different.

2) If Ishaan is working on a project with Harry I agree that the ART repo should be set up the same but why should the place where the ART repo be the same? That is the same as saying if me and Harry are working on a project that uses a computer, both our computers should be set up the same. I think using the same repository is more than enough and anything additional is more restriction

AaronYoung5 commented 10 months ago
  1. Its easier for to access data if the OS gets corrupted and is on a separate SSD (or) if I want to change OS (not sure if it applies here), I can do it without worrying about my data being lost. I like having the comfort of having my data in a place that is separate from the OS because the OS is subject to change. The setup scripts change because all the paths are different.
  2. If Ishaan is working on a project with Harry I agree that the ART repo should be set up the same but why should the place where the ART repo be the same? That is the same as saying if me and Harry are working on a project that uses a computer, both our computers should be set up the same. I think using the same repository is more than enough and anything additional is more restriction
  1. eMMC load speeds for the onboard card is about 300 MB/s but a standard NVME M.2 can be well above 1000, depending on the model you have, upwards of 3000 MB/s, which is significant. Any read/writes to the eMMC will degrade performance. I personally don't see data corruption as an issue; this happened previously due to storage limits, which won't be an issue with the nvme card.

  2. Maybe what I'm saying is unclear. The script runs on the vehicle, not your computer. Does the clear things up? In your example, it's not that both your computers should be setup the same, the shared computer should be setup the same because we all share those computers. I see that it's in a sense "restrictive" in that it must be in this location, but it's not restrictive in the sense that it stops you from developing at all. It's just for consistency across computers.

But from a higher level, these changes are based on previous experience with the platforms. We keep rewriting/overwriting code. These measures are being made in hopes of reducing code loss when a lab member leaves, improve interoperability between computers/users, and make things simpler.

Huzaifg commented 10 months ago
  1. That makes sense. I was not aware of the speed difference. We will make the change, but not right now as it is not our first priority - we are not seeing any problems associated with the speed difference. When we do see a problem, we will make the change - either way its making the change only once and I don't want to put a spoke in the wheels right now.

  2. I don't see the computers as shared. The script runs on the computer on the vehicle. The vehicle only listens to Arduino commands. The computer account sbel is meant to be used by only Ishaan and I. It does not have to be general enough in my opinion to be used by whoever is in the lab. This is highly restrictive, I don't want to think about rules when I am doing simple things like placing a folder somewhere. I am sure if Harry uses nannyberry, he will be able to figure out where the repository is. The vehicle specific services make sense and will be done, but I do not agree that anything more than that is required. We are anyways adding another layer of abstraction of the OS on top of this which gets everything set up exactly the way its needed for ART.

StefanCaldararu commented 10 months ago
  1. I don't see the computers as shared. The script runs on the computer on the vehicle. The vehicle only listens to Arduino commands. The computer account sbel is meant to be used by only Ishaan and I. It does not have to be general enough in my opinion to be used by whoever is in the lab. This is highly restrictive, I don't want to think about rules when I am doing simple things like placing a folder somewhere. I am sure if Harry uses nannyberry, he will be able to figure out where the repository is. The vehicle specific services make sense and will be done, but I do not agree that anything more than that is required. We are anyways adding another layer of abstraction of the OS on top of this which gets everything set up exactly the way its needed for ART.

Hmm I actually agree with @AaronYoung5 on this one that consistency is better. And as an anecdotal example, not having this consistency HAS caused issues in the past (on chokecherry people were cloning the same repository in multiple locations on the same machine - that makes no sense). When it comes to doing development that involves the Jetson you can do whatever you want anywhere on the computer, but imo if we are doing autonomy-research-testbed specific hardware tests, i.e. using the Jetson, autonomy stack, and physical vehicle, it would be nice to keep stuff as consistent as possible. That way when someone gets onboarded they can learn one consistent way for how to use the hardware...

@Huzaifg can you please explain a bit more how this is restrictive?

StefanCaldararu commented 10 months ago

Especially with the discussion for MaGIC tutorials, it makes a lot more sense to have the setup for the hardware / software infrastructure to be somewhat consistent, with modular parts (sensor suites)

Huzaifg commented 10 months ago

The hardware and software is already consistent with the docker containers. There is no point in making it consistent even outside the container. It is restrictive because it adds one more thing to the already extremely long list of things needed to work with this repository.

With respect to people cloning multiple repositories, well, that is the fault of the people using the machine. I don't think we should teach new people how to follow very specific steps to use this repository. They should understand the high level idea: which is 1) Clone the repository where ever you like 2) Build the container using ATK. 3) Within the container (where everything needs to be consistent and is), colcon build.

The more restrictions the more "do this or it will not work - don't bother about why we do this".

PS: The hardware / software is already somewhat consistent - somewhat consistent does not mean exactly the same.

StefanCaldararu commented 10 months ago

The hardware and software is already consistent with the docker containers. There is no point in making it consistent even outside the container. It is restrictive because it adds one more thing to the already extremely long list of things needed to work with this repository.

With respect to people cloning multiple repositories, well, that is the fault of the people using the machine. I don't think we should teach new people how to follow very specific steps to use this repository. They should understand the high level idea: which is

  1. Clone the repository where ever you like
  2. Build the container using ATK.
  3. Within the container (where everything needs to be consistent and is), colcon build.

The more restrictions the more "do this or it will not work - don't bother about why we do this".

PS: The hardware / software is already somewhat consistent - somewhat consistent does not mean exactly the same.

@Huzaifg Have you tried using the setup tool @AaronYoung5 set up? It's actually less steps than it was before... Instead of setting up:

You just run the script Aaron made and it sets everything up for you. The rest is so that you can push commits from the Jetsons, which is DEFINITELY necessary.

With respect to people cloning multiple repositories, well, that is the fault of the people using the machine.

I agree, but I don't think your solution of "no one other than me and Ishaan can use the machine" is the right solution to this. And it is annoying to have this happen, no matter who's fault it is.

The more restrictions the more "do this or it will not work - don't bother about why we do this".

I wouldn't say these are restrictions - it is still possible to set up the repo as you did before in the same way, and it will still work. It's just nice to have a bash script which sets everything up in the same way...

Huzaifg commented 10 months ago

That's what I am saying, we are not going to use the bash script to set everything up again because it is already set up.

I personally learnt a lot by setting it up myself. Especially such basic things - it helps understand a bit how things are connected in ART (very little because there are alot of other things abstracted out) which I think is absolutely necessary for a user.

Other people can use our Jetson, but do not expect it to be as consistent as having all the folders in the same place, that is not possible.

If I am a new student wanting to learn how ART works, I would rather try setting up the basic building blocks myself rather than run a bash script and read what the bash script does. That way, if something goes wrong, I will have some chops to debug it myself rather than raising an issue "setup script does not work".

Also, are you going to have another setup script for computers other than Jetson? What if I use a jetson nano or a nuc? Will we have different setup scripts for that? What if an external user has a computer completely different from what we have used, should we write a setup script for that as well?

My point being - we cannot teach people to just run the setup script and we should also not expect all users to always use the setup script. We should rather leave stuff that you do in the setup script to the user. The user should only follow instructions from this repo when they enter the repo. I have no problem providing a setup script, but enforcing that everyone uses it is in my opinion not the way to go.

PS : The setup to push commits is nice to have but it is not the most pressing problem right now :)

StefanCaldararu commented 10 months ago

That's what I am saying, we are not going to use the bash script to set everything up again because it is already set up.

As in it has diverged from the rest of the repo? if that is the case that is a bad thing. Otherwise it wouldn't be hard at all, and would ensure you have pre-commit set up, can push to source, etc.

If I am a new student wanting to learn how ART works, I would rather try setting up the basic building blocks myself rather than run a bash script and read what the bash script does.

Yes, I agree with this, but this is not consistent with you previous point. If you are a student interested in control algorithms, then maybe you don't care about framework setup and would rather just have it be done. As I said, if one wants to they can just directly clone the repo and follow the atk instructions. If they are interested in learning about the framework that is definitely encouraged (maybe the docs don't reflect that right now?).

PS : The setup to push commits is nice to have but it is not the most pressing problem right now :)

I strongly disagree with this. This, along with not having a consistent setup encourages the repo divergence I mentioned above... which is something that is VERY difficult to recover from (from personal experience).

I guess my main point is this: There are two cases. One is you are interested in the framework and learning about it. The other is that you are interested only in the algorithm / autonomy / sim development. But neither seems like a good reason to say "I'm going to use my own framework that no one else uses..."

Huzaifg commented 10 months ago

@StefanCaldararu

Not using the setup script does not mean our branch has "diverged" from main. As I have said, the vehicle specific services make sense (although the issue did not explain why this was done - maybe it was one of the other issues that I might have missed), this will be done. My point is with the even more specific requirements of having the art repo in home and having home on the external SSD. 1) I am convinced that having home (and the OS) on the external SSD is a good step - although it is NOT pressing. The performance drop is NOT a problem for us at the moment and the loss of time in doing this is TOO high right now - it will be done in due course, when the performance drop IS an issue. 2) The ART repo being in the home folder cannot be a requirement of using ART (and should not be). I cannot guarantee that it will be in home, because such guarantee's should not be made.

Setting up the push commits does not in any way relate to having repo divergence. All it does is to have my username associated with a particular commit I make on Nannyberry. This is NOT pressing because me and Ishaan already have an understanding of how we are going to go about this. Additionally, we DO NOT want anyone to push from the ART repo on Nannyberry. This HAS to go through us and SHOULD use our passwords.

Again, I am not saying "I am going to use my own framework", I am using ART but the way I use ART is up to me. This holds for all packages, the developers of a package have no business constraining where the source code is placed on a machine.

And again, the setup script is nice to have (for someone who only cares about writing a control node), but is not something that should be enforced

StefanCaldararu commented 10 months ago

My point is with the even more specific requirements of having the art repo in home and having home on the external SSD.

@Huzaifg I agree with you about this, that this shouldn't be a requirement. Whether or not it is a good thing to do (and something this is done) is irrelevant of it being required. I agree it shouldn't be required. (I will let @AaronYoung5 comment on this more).

Setting up the push commits does not in any way relate to having repo divergence. All it does is to have my username associated with a particular commit I make on Nannyberry. This is NOT pressing because me and Ishaan already have an understanding of how we are going to go about this. Additionally, we DO NOT want anyone to push from the ART repo on Nannyberry. This HAS to go through us and SHOULD use our passwords.

I still disagree with this... Here is a concrete example: I see that the last commit on your branch is titled IMU GPS launch file, I am assuming this sets up the RTK GPS and IMU (I haven't actually read through the commit, so sorry if I get some stuff wrong). Harry and I would like to use the IMU and RTK GPS. I have no clue how to set up the RTK GPS, and I am not interested in learning (personally). Wouldn't it make sense to have a consistent way to set up this sensor, and once someone figures it out (I am guessing your vehicle is set up with this), it can go into the master branch and anyone can use it? Or do you think I should have to set up the RTK GPS from scratch, potentially in a different way (what I call divergence)? Or is there some other suggestion for this that I am missing?

Yes, to achieve this you don't have to have your vehicle set up with the setup script. But then if you want to share the RTK GPS setup with the rest of us, you have to have the required dependancies to merge into master (pre-commit). Do you have that set up? This would be another thing in the "endless list of setup" that the setup script would be able to automate.

Additionally, we DO NOT want anyone to push from the ART repo on Nannyberry. This HAS to go through us and SHOULD use our passwords.

I don't understand this point; If I can access a machine, I can clone / pull / push to a repo how ever I want, and there isn't really a way to prevent this. the sbel-ssh-add aliases ensure that I can't push as YOU. If I am able to access the machine and you have an id_rsa file without a password, I WILL be able to push as you, whether or not you want me to. The solution to this is:

1) I don't have access to the machine (not a good solution IMO) 2) You have passwords on you rsa files (this is what gets aliased by sbel-ssh-add)

but going back to the previous point, what do you suggest for the concrete example I showed? What is the best way for me to set up the RTK GPS given that you guys have already set it up?

p.s. sorry for closing and reopening, I like pressing random buttons :)

AaronYoung5 commented 10 months ago

Thanks guys for the discussion, like I really wish we have more discussions like this cause otherwise I don't know what people are thinking.

@Huzaifg The location of the source code is not a requirement of anything (nor is the setup script), it's a convenience for us as a team to collectively develop art/cobra/research together. We (like you, me, Ishaan, Harry, etc.) are the developers and users of the package, and we as a team (i.e. SBEL) should decide as a group what the best course of action is. And so irrespective about whether I agree with you about how we should develop our stuff, I just want to be on the same page. At the end of the day, even this repo isn't required, but we should all agree that what is here makes the most sense (or change it, if not). <- not saying what is here makes the most sense, but that we should strive to do things that make the most sense.

And so I certainly agree with many of your points. Especially that people don't have an underlying understanding of a framework if they just blindly set things up (like run the script, use atk, etc.) - I/others have to then help others, taking up time, etc... And it's something I've been thinking about lately cause I've been answering a lot of duplicate questions (hence the docs, not sure if it helps much). Maybe if you have some other suggestions, I'd love to hear them.

So at the end of the day, I certainly can't, nor want to, "enforce" anything. But we should discuss (like this) the best course of action, agree on something, and do that. I wrongly assumed others would agree that the script made sense as a group to use, and so more discussion here in the future would be helpful!! I post here (github) quite often, so I hope more people will be apart of the discussion in the future. We can talk more at the meeting this afternoon (if we're still having it, not really sure)

Huzaifg commented 10 months ago

@StefanCaldararu

  1. By "setting up push commits" I meant the key stuff. Sorry for being unclear. I completely agree with what you said. I was just arguing that setting up the ssh key is not related to what you just described. Dosen't the vehicle specific setup ensure that? If there is something I missed, please let me know.
  2. For the ssh key, I get your point. I just have a concern with people commit-ing stuff on nannyberry-art without going through us. But I guess there is no way of preventing that. We will set up the sbel-ssh key
Huzaifg commented 10 months ago

@AaronYoung5

I think it would help to add a few lines about atk to the homepage of the ART repo? Also maybe a link to the docs folder? I can do part 2, but part 1, I don't know enough about atk

StefanCaldararu commented 10 months ago
  1. For the ssh key, I get your point. I just have a concern with people commit-ing stuff on nannyberry-art without going through us. But I guess there is no way of preventing that. We will set up the sbel-ssh key

This is a valid concern. I think what you mean is that you and Ishaan have your own development branch and you don't want other people messing around with it, correct? My big concern is still the divergence. I am realizing I am probably not doing the best job of explaining it, so let's maybe talk later today and I will try and think of a good way to explain it in text, and then post here after the discussion. Sounds good @Huzaifg ?

Huzaifg commented 10 months ago

@StefanCaldararu

Yes that is the concern. Lets talk maybe tomorrow? I will also get Ishaan in the loop so that we are on the same page. Thanks!!