Skip to content

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

Merged
merged 66 commits into from
Jul 8, 2020

Conversation

liljonnystyle
Copy link
Collaborator

@liljonnystyle liljonnystyle commented May 22, 2020

Pull request information

  • Status: ready to merge
  • Kind of changes: new feature

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.

@liljonnystyle liljonnystyle changed the base branch from master to i210_dev May 22, 2020 05:29
joyncarpio and others added 5 commits May 26, 2020 23:55
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.
liljonnystyle and others added 7 commits May 27, 2020 14:01
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
Copy link
Collaborator Author

@liljonnystyle liljonnystyle left a 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() and get_power()
  • get_power() needs to be added to the base vehicle class as an abstractmethod
  • docstrings need to be updated
  • some minor stylistic changes

Copy link
Member

@kjang96 kjang96 left a 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

Copy link
Member

@kjang96 kjang96 left a 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

Copy link
Member

@kjang96 kjang96 left a 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!

@liljonnystyle liljonnystyle merged commit 7c9a48a into i210_dev Jul 8, 2020
@liljonnystyle liljonnystyle deleted the jc-energy-class branch July 8, 2020 18:10
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.

5 participants