-
Notifications
You must be signed in to change notification settings - Fork 385
Energy class for inventorying multiple energy models #944
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
Conversation
This python file describes the base energy file which will serve as the parent class of the power demand model and toyota energy models.
applied comments from from previous PR.
These are child classes of the base_energy class.
Some imports removed
added space after all commas
this code is actually unrelated to the energy classes being implemented, but rather the fuel consumption from SUMO
Updated traci.py, params.py and rewards.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good, mostly pretty minor changes now:
- all the changes to traci.py can be reverted, except for
_add_departed()
andget_power()
get_power()
needs to be added to the base vehicle class as an abstractmethod- docstrings need to be updated
- some minor stylistic changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good work! I tried out some examples and they work without a hitch. I left a few review notes. Look over Jonny's notes as well, and we're almost there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple more nits. Also please make sure you run the code for all energy models and make sure they're working. By this, I mean, go into exp_configs, and make sure that the code runs when you add vehicles of all the kinds of energy models that you have specified
…into jc-energy-class
…jc-energy-class
…jc-energy-class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once this pass is done and the build passes, LGTM!
Pull request information
Description
This is a new implementation of a base energy class along with a few child classes to help us inventory the multiple energy models we plan to have.
@liljonnystyle is uploading on behalf of @joyncarpio to initiate tracking of code changes.