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

Electric Vehicle Model: Internal moment of inertia power calculated incorrectly #12513

Closed
abrac opened this issue Jan 22, 2023 · 11 comments
Closed
Assignees
Milestone

Comments

@abrac
Copy link

abrac commented Jan 22, 2023

Hi there. As stated in the docs, there is a model parameter called internalMomentOfInertia. This parameter, I, has units: kg·m². The correct way of calculating the power required for generating rotational inertia is:

P = [ ½ * I/r_wheel^2 * (v^2 - v_old^2) ] / Δt

According to HelpersEnergy.cpp#L65, it is calculated differently... Instead of the factor I/r_wheel^2, it simply uses I.

On the other hand, the new MMPEVEM model calculates it correctly, as show in HelpersMMPEVEM.cpp#L83.

According to my calculations, SUMO's internalMomentOfInertia default, seems incorrect... The default is 0.01 kg·m². On the other hand, the values in the example configurations of MMPEVEM are in the magnitude of 12--16 kg·m², which are much closer to my calculations...

Something seems to be wrong somewhere with the modeling.

SUMO-version: 1.15

operating system: N/A

@namdre
Copy link
Contributor

namdre commented Jan 23, 2023

Thank you for looking at this in detail!

  1. The low value of 0.01 comes via bad default values for electricity consumption model parameters #5505 and I'm in doubt that the cited reference is correct since there are no numbers in this publication. Our nightly tests (presumably for electric buses) uses a value of 30.
  2. Looking at HelpersMMPEVEM.cpp#L83, I cannot find the part corresponding to (v^2 - v_old^2) / Δt

@m-kro
Copy link
Contributor

m-kro commented Jan 23, 2023

@namdre wouldn't that be part of a (see input parameters of calcPowerConsumption())? Probably due to the different scale of the models...

@abrac
Copy link
Author

abrac commented Jan 23, 2023

I assume that it is part of a as @m-kro has said. However, it may be important to note that MMPEVEM uses a slightly different approach. They calculate P using the following equation:

P = F v = T/r_wheel v = (α I)/r_wheel v = (a I)/r_wheel² v

@abrac
Copy link
Author

abrac commented Jan 23, 2023

The publication (Open access version) from the developers of the MMPEVEM model does report their model parameter of 12.5 kg·m², and they seem to verify their results using a dynamometer. So maybe the SUMO default could be changed to that. In addition, the default SUMO EV model needs to have its inertia divided by r_wheel² in HelpersEnergy.cpp#L65. I think if these two changes are implemented, the default model would output a more accurate result, which would correspond with that of the MMPEVEM model... I wonder if @kevbad (co-author of MMPEVEM) has any insights on this...?

@abrac
Copy link
Author

abrac commented Jan 23, 2023

@kevbad This could also be one of the reasons that you found the SUMO result to be consistently less than your model's results. Since the sumo model did not divide the inertia by r_wheel², the power calculation would have been underestimated.

@kevbad
Copy link
Contributor

kevbad commented Jan 23, 2023

Hello everyone,

in HelpersMMPEVEM.cpp, we calculate the mass factor of the vehicle which allows us to take rotating parts into account when using formulas for translational movement. Please note that we also divide by the vehicle mass and not just by the square of the wheel’s radius.
I agree with @abrac that this isn’t implemented correctly in HelpersEnergy.cpp. Either the authors forgot to divide by m∙r² or they meant to multiply by the angular velocity and forgot to convert v.

@namdre namdre added this to the 1.16.0 milestone Jan 30, 2023
@namdre namdre assigned m-kro and unassigned behrisch Jan 31, 2023
@behrisch
Copy link
Contributor

behrisch commented Feb 5, 2023

To add a little bit to the confusion, the ElectricHybrid docs state that they use the internalMomentOfInertia as a mass and not as a moment.

@abrac
Copy link
Author

abrac commented Feb 7, 2023

To add a little bit to the confusion, the ElectricHybrid docs state that they use the internalMomentOfInertia as a mass and not as a moment.

Indeed. The authors of ElecHybrid obviously noticed that HelpersEnergy does not divide I by r^2. Therefore they redefine I as the "equivalent mass" that would compensate for the inertia. This equivalent mass is ordinarily calculated by dividing I by r^2. But since, I is not divided by r^2, they just redefined I as the equivalent mass.

@m-kro m-kro modified the milestones: 1.16.0, 1.17.0 Feb 14, 2023
@palvarezlopez palvarezlopez modified the milestones: 1.17.0, 1.18.0 Apr 25, 2023
@m-kro
Copy link
Contributor

m-kro commented Jun 12, 2023

After consulting the thesis of the model author Kurczveil, Tamás (2017): Optimierung einer induktiven Energieversorgungsinfrastruktur für den urbanen Straßenverkehr. (p. 95), it seems the parameter ist just wrongly named "internal moment of inertia" and the default value & unit is questionable, too. From the publication, it should have read something like "equivalent mass for internal rotating elements".

@m-kro
Copy link
Contributor

m-kro commented Jun 15, 2023

Closing here due to the upcoming release. The doc update now reflects more the original model definition and the physical relations. Discussions about the default values of battery device etc. continue in #13422 .

@m-kro m-kro closed this as completed Jun 15, 2023
behrisch added a commit that referenced this issue Apr 3, 2024
behrisch added a commit that referenced this issue Apr 3, 2024
@behrisch
Copy link
Contributor

behrisch commented Apr 4, 2024

I changed the name of the param to "rotatingMass" now for the Electric and Hybrid models and changed the default to 40 (which is the value PHEMlight uses for its battery vehicle). @abrac @kevbad If you want to change something please speak up before the release is out in two weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants