Motor losses #36

Harsha-Samavedam · 2026-01-17T21:35:36Z

calculating electrical motor losses

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

gemini-code-assist · 2026-01-17T21:36:01Z

Summary of Changes

Hello @Harsha-Samavedam, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces significant advancements to the vehicle simulation framework by integrating detailed electrical motor and battery loss models, refining existing energy models for solar array and rolling resistance, and establishing a robust data logging and visualization pipeline. It culminates in the creation of an interactive Streamlit UI, making the simulation more accessible and user-friendly for analysis and parameter tuning.

Highlights

Motor and Battery Loss Models: Introduced comprehensive models for calculating electrical motor losses (MotorLossModel in motor_losses.py) and battery Equivalent Series Resistance (ESR) losses (ESRBatteryLossModel in battery.py), allowing for more accurate energy consumption simulations.
Simulation Framework Enhancements: Refactored the core simulation logic by splitting base.py into energy_model.py (for the abstract EnergyModel) and vehicle_model.py (for the VehicleModel), improving modularity. The VehicleModel now specifically integrates battery models.
Advanced Array and Rolling Resistance Models: The SCPArrayModel now includes a variant (SCPArrayModelWithIncidence) that accounts for the sun's incidence angle, and the SCPRollingResistanceModel has been updated to differentiate between front and rear wheel weights for more precise calculations.
Data Logging and Visualization: The main.py script has been significantly upgraded to support command-line arguments for selecting parameters to log, specifying CSV output filenames, and generating time-series graphs using matplotlib and pandas. This provides flexible data analysis capabilities.
Interactive User Interface: A new Streamlit-based web application (ui.py) has been added, offering an interactive way to configure simulations, visualize results with plotly graphs, and download data, greatly enhancing the user experience.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request significantly enhances the vehicle simulation by adding a command-line interface, data logging to CSV, and graph generation. It also introduces new models for battery and motor losses, and refactors the existing model structure. The changes are extensive and add a lot of value. However, there are several critical issues that will prevent the code from running, mainly related to incorrect imports after file renaming and missing model initializations that will lead to runtime errors. There are also some medium-severity issues regarding incorrect type hints and magic numbers. I've provided detailed comments and suggestions to address these problems.

I am having trouble creating individual review comments. Click here to see my feedback.

main.py (178-180)

A BatteryModel is not being added to the VehicleModel. The updated VehicleModel.update method expects a battery model to be present to calculate battery losses. To include these calculations in the simulation, you should add the BatteryModel. This will also require importing it at the top of the file: from models.battery import BatteryModel.

models/array.py (69)

This line contains an indented comment, which is a SyntaxError in Python and will cause the program to crash if this code path is executed. Please unindent the comment.

        # energy produced this timestep (Power × time)

models/motor.py (1)

The import from models.base import ModelPlugin is incorrect. The file models/base.py has been renamed to models/vehicle_model.py, and it does not contain a ModelPlugin class. This will cause an ImportError. This file also appears to be incomplete.

models/vehicle_model.py (23)

This line will raise an AttributeError if no BatteryModel has been added to the VehicleModel, as self.battery_model will be None. You should add a check to ensure self.battery_model exists before calling its update method to make the model more robust.

        if self.battery_model:
            self.params['total_energy'] += self.battery_model.update(self.params, self.params['timestep'])

ui.py (7)

The import from models.base import VehicleModel is incorrect because models/base.py was renamed to models/vehicle_model.py. This will cause an ImportError when running the UI.

from models.vehicle_model import VehicleModel

ui.py (71-73)

A BatteryModel is not being added to the VehicleModel. The updated VehicleModel.update method expects a battery model to be present to calculate battery losses. To include these calculations in the simulation, you should add the BatteryModel. This will also require importing it at the top of the file: from models.battery import BatteryModel.

main.py (25)

The function's return type hint dict[str, Quantity[float]] does not match the implementation. The result dictionary can contain datetime objects, as handled in the if block. The type hint should be updated to reflect this to improve code clarity and help static analysis tools.

def parse_yaml(yaml_path: str) -> dict[str, datetime | Quantity[float]]:

models/battery.py (10)

The type hint for the params dictionary is dict[str, float], but the VehicleModel passes a dictionary of pint.Quantity objects (dict[str, Quantity[float]]). Using the correct type hint will improve code clarity and allow static analysis tools to catch potential errors.

    def update(self, params: dict[str, Quantity[float]], timestep: Quantity[float]) -> Quantity[float]:

models/battery.py (27)

The type hint for the params dictionary is dict[str, float], but it should be dict[str, Quantity[float]] to match the data passed by VehicleModel. This will improve type safety and code clarity.

    def update(self, params: dict[str, Quantity[float]], timestep:Quantity[float]):

models/motor_losses.py (116)

The magic number 9.549 is used for converting power from (N·m * RPM) to Watts. It would be better to define this as a named constant with a comment explaining its origin (60 / (2 * pi)) to improve readability and maintainability.

models/motor_losses.py (242)

The value 3000 appears to be a reference RPM and is hardcoded. This makes the calculation less flexible and harder to understand. Consider defining it as a named constant or passing it as a parameter to the function.

IshDeshpa · 2026-01-22T18:49:47Z

Currently the UI stuff is adding too much to this PR. I'd like you to move everything UI related to a different branch, then I'll start taking a look at the motor loss stuff to verify it.

Additionally, I don't think ModelPlugin exists as a class. Is this a hallucination perhaps?

- Resolved import conflict in main.py - Added LVDrawModel import - Kept BatteryModel initialization

IshDeshpa

tsk tsk tsk

.gitignore

models/motor_losses.py

main.py

pyproject.toml

ui.py

output/drag_force_graph.png

output/frontal_area_graph.png

IshDeshpa

tsk tsk tsk 2

ui.py

models/motor_losses.py

.gitignore

main.py

IshDeshpa

getting closer...

IshDeshpa · 2026-01-31T20:59:01Z

models/motor_losses.py

+    ) -> PlainQuantity[float]:
+        # Get current operating conditions from params
+        # These should be set by the simulation based on current state
+        I = params.get('motor_current', Q_(0, 'A'))


Use the drag_power and rr_power added together to derive the current here. Should be:

drag_power + rr_power / battery voltage

IshDeshpa · 2026-01-31T20:59:21Z

models/motor_losses.py

+        # Get current operating conditions from params
+        # These should be set by the simulation based on current state
+        I = params.get('motor_current', Q_(0, 'A'))
+        N_rpm = params.get('motor_speed', Q_(0, 'rpm'))


Calculate this from the velocity parameter (divide by wheel circumference)

models/motor_losses.py

.gitignore

models/rr.py

models/array.py

IshDeshpa

getting better...

IshDeshpa · 2026-01-31T21:44:11Z

models/motor_losses.py

+    ) -> PlainQuantity[float]:
+        # Calculate motor speed from velocity and wheel circumference
+        # N_rpm = (velocity / wheel_circumference) * 60
+        import numpy as np


Shouldn't need to import this; already imported at the top of the file

IshDeshpa · 2026-01-31T21:45:39Z

models/motor_losses.py

+        velocity = params.get('velocity', Q_(0, 'm/s'))
+        wheel_diameter = params.get('wheel_diameter', Q_(0.5, 'm'))  
+        wheel_circumference = np.pi * wheel_diameter
+        # Convert velocity to rotational speed
+        N_rpm = (velocity / wheel_circumference * Q_(60, 's/min')).to('rpm')
+
+        # Calculate motor current from power demands
+        # I = (drag_power + rr_power) / battery_voltage
+        drag_power = params.get('drag_power', Q_(0, 'W'))
+        rr_power = params.get('rr_power', Q_(0, 'W'))


Instead of using params.get here, I would probably actually recommend using the typical params['key'] method of getting values from a dictionary so that it errors out if the key doesn't exist. This way we don't keep running the sim with a bogus value if the key doesn't exist in params.yaml.

IshDeshpa · 2026-01-31T21:48:10Z

params.yaml

+  name: motor_eta_C_default
+  unit: dimensionless
+  value: 0.97
+-
+  name: motor_T_ref_default
+  unit: °C
+  value: 25.0
+-
+  name: motor_T_operating_default
+  unit: °C
+  value: 70.0
+-
+  name: motor_k_H_default
+  unit: W/rpm
+  value: 0.0
+-
+  name: motor_k_E_default
+  unit: W/rpm^2
+  value: 0.0
+-
+  name: motor_k_B1_default
+  unit: N·m
+  value: 0.0
+-
+  name: motor_k_B2_default
+  unit: N·m·min/rad
+  value: 0.0
+-
+  name: motor_k_D_default
+  unit: W/rpm^3
+  value: 0.0
+-
+  name: motor_k_SW_default
+  unit: W/rpm
+  value: 0.0
+-
+  name: motor_k_1_default
+  unit: W/rpm
+  value: 0.0
+-
+  name: motor_k_2_default
+  unit: W/rpm^2
+  value: 0.0
+-
+  name: motor_k_3_default
+  unit: W/rpm^3
+  value: 0.0
+
+# Motor parameters - general configuration for the vehicle's motor
+-
+  name: motor_k_T
+  unit: N·m/A
+  value: 0.1910
+-
+  name: motor_k_C
+  unit: V·min/rad
+  value: 0.020
+-
+  name: motor_k_S
+  unit: N·m/A
+  value: 0.1910
+-
+  name: motor_R_A
+  unit: ohm
+  value: 0.0931
+-
+  name: motor_R_B
+  unit: ohm
+  value: 0.0233
+-
+  name: motor_eta_C
+  unit: dimensionless
+  value: 0.97


i would put some comments here explaining what these constants are (maybe grab some info from the confluence doc you made).

models/motor_losses.py

params.yaml

IshDeshpa

It seems like the only function you actually need is total_motor_loss. I would remove anything that's unnecessary.

IshDeshpa · 2026-02-07T20:03:09Z

models/motor_losses.py

+    def R_equivalent_battery_side(self, params: dict[str, PlainQuantity[float]]) -> PlainQuantity[float]:
+        eta_C = params['motor_eta_C']
+        return 1.5 * self.R_total(params) / eta_C


Don't know if this function is actually being used.

IshDeshpa · 2026-02-07T20:04:44Z

models/motor_losses.py

+        wheel_diameter = params['wheel_diameter']
+        wheel_circumference = np.pi * wheel_diameter
+        # Convert velocity to rotational speed
+        N_rpm = (velocity / wheel_circumference * Q_(60, 's/min')).to('rpm')


You can remove the * Q_(60, 's/min') since you're using .to('rpm') anyways.

IshDeshpa · 2026-02-07T20:12:48Z

models/motor_losses.py

+        N_rpm = (velocity / wheel_circumference * Q_(60, 's/min')).to('rpm')
+
+        # Calculate motor current from power demands
+        # I = (drag_power + rr_power) / battery_voltage


remove comment

IshDeshpa · 2026-02-07T20:16:08Z

models/motor_losses.py

+        # Convert (N·m × rpm) to Watts: 9.549 = 60/(2π) for rpm to rad/s conversion
+        P_shaft = tau_S * N_rpm / Q_(9.549, 'dimensionless')


Can you not just do .to('Watts') here?

IshDeshpa · 2026-02-07T20:17:57Z

models/motor_losses.py

+            'eta_controller': eta_C
+        }
+
+    def system_power_analysis(self, tau_S: PlainQuantity[float], N_rpm: PlainQuantity[float], V_bus: PlainQuantity[float], params: dict[str, PlainQuantity[float]]) -> Dict[str, PlainQuantity[float]]:


When is this actually called?

IshDeshpa · 2026-02-07T20:18:45Z

models/motor_losses.py

+            'P_motor_total': P_total
+        }
+
+    def motor_efficiency(self, tau_S: PlainQuantity[float], N_rpm: PlainQuantity[float], params: dict[str, PlainQuantity[float]]) -> Dict[str, PlainQuantity[float]]:


if system power analysis is never called then this function is not needed either

Harsha Samavedam and others added 19 commits December 2, 2025 16:04

Added logging at all specified timesteps

a047e6c

added a few comments

4ac3cce

updated time steps and added run-simulation method

a810e09

added graphing for specified parameters

82db0be

updated date and made logging command simpler

e27c260

Update main.py

dc081ed

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Apply suggestions from code review

52e922b

removed cached fiels

d660c22

Added logging at all specified timesteps

9b5ed1d

added a few comments

9489c26

updated time steps and added run-simulation method

369b8ae

added graphing for specified parameters

543024b

updated date and made logging command simpler

4b62cc3

Update main.py

bb3ce8e

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Apply suggestions from code review

bcf178f

removed cached fiels

b372eb8

default log case

8cc0c31

added default log case

a925de6

electrical motor losses

060da7f

Merge branch 'CSV-logging' into motor_losses

43fd7ee

gemini-code-assist bot reviewed Jan 17, 2026

View reviewed changes

IshDeshpa self-requested a review January 22, 2026 18:43

IshDeshpa changed the base branch from CSV-logging to main January 22, 2026 18:44

Harsha Samavedam and others added 5 commits January 24, 2026 12:24

remove ui files from branch and old motor model

404f0c1

Remove motor.py from tracking

b4d11ec

Merge branch 'main' into motor_losses

eb5ecfc

Merge branch 'main' into motor_losses

5bc0a5d

- Resolved import conflict in main.py - Added LVDrawModel import - Kept BatteryModel initialization

moved params to yaml

5136d99

Harsha-Samavedam force-pushed the motor_losses branch from 0c1ac7c to 5136d99 Compare January 31, 2026 17:03

removed try catch

557e832

IshDeshpa requested changes Jan 31, 2026

View reviewed changes

Harsha Samavedam added 2 commits January 31, 2026 11:54

add output/ to gitignore and untrack output files

a8a1ccf

fixed pr comments

d9cbd26

IshDeshpa requested changes Jan 31, 2026

View reviewed changes

Harsha Samavedam added 3 commits January 31, 2026 14:16

removed assert

f2ee971

fixed comments again

650d29e

Merge remote-tracking branch 'origin/main' into motor_losses

d767391

IshDeshpa requested changes Jan 31, 2026

View reviewed changes

tsk tsk tsk v3

e4caaf1

IshDeshpa requested changes Jan 31, 2026

View reviewed changes

Harsha Samavedam added 2 commits January 31, 2026 16:11

fixed comments round 4

f83f369

fixed comments round 4

d8cdf5c

IshDeshpa requested changes Feb 7, 2026

View reviewed changes

		# Convert (N·m × rpm) to Watts: 9.549 = 60/(2π) for rpm to rad/s conversion
		P_shaft = tau_S * N_rpm / Q_(9.549, 'dimensionless')

Motor losses #36

Are you sure you want to change the base?

Motor losses #36

Uh oh!

Conversation

Harsha-Samavedam commented Jan 17, 2026

Uh oh!

gemini-code-assist bot commented Jan 17, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

main.py (178-180)

models/array.py (69)

models/motor.py (1)

models/vehicle_model.py (23)

ui.py (7)

ui.py (71-73)

main.py (25)

models/battery.py (10)

models/battery.py (27)

models/motor_losses.py (116)

models/motor_losses.py (242)

Uh oh!

IshDeshpa commented Jan 22, 2026

Uh oh!

IshDeshpa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IshDeshpa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IshDeshpa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IshDeshpa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

IshDeshpa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment