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

Refactor wire loss and vehicle battery charging #64097

Merged
merged 12 commits into from
Mar 20, 2023

Conversation

irwiss
Copy link
Contributor

@irwiss irwiss commented Mar 10, 2023

Summary

None

Purpose of change

Fixes #64069 - negative value no longer fed into ammo_set resulting in full batteries.

d247281 fixes a bug introduced in #61950 - loss switched to kilowats, json stayed in watts so currently there's no loss data on any wires.

Fixes wire loss calculation, so requesting 1,000,000kJ with 1% loss factor but 500kJ in available charges will not result in a 10000kJ wire loss - instead only discharged amount.

Describe the solution

Batteries are now assumed to be connected "in parallel", batteries equalize their charge by percentage, extra batteries connected are adding capacity at same voltage, charge is no longer "donated away" when local ones full, instead all connected batteries are considered part of same electric system. Effects not simulated: time to make equalization transfer happen, needing a resistor/load so batteries don't explode if you connect empty battery to full ones, battery heat during transfers, may be more.

Simplifies battery and fuel functions - they no longer do recursion, instead search_connected_vehicles / search_connected_batteries return flattened maps with minimum sum of wire loss to battery as value, this allows removing "recursion" from functions that don't do recursion except as a special case for batteries, like vehicle::fuel_left and similar.

This PR fixes how line loss is calculated, so requesting a drain of 1,000,000kJ with just 500kJ in the battery will no longer produce a -9995kJ discharge to wire loss, instead loss is calculated for the amount drained. Also prevents feeding negative values into ammo_set when activating things like water purifiers resulting in (seemingly randomly) fully charging batteries.

Wire loss is now calculated as weighted average with weight set to ( battery_capacity * sum_of_wire_losses_to_battery )

Minor details:
Now uses dijkstra instead of BFS (to do weighted graph)
Possible (but not yet observed) performance regressions can be solved by caching connected vehicles graph similar to how parts are cached

Describe alternatives you've considered

Axing power loss entirely... It's relative to current squared and linear to resistance, so the higher the voltage the less loss there is; It's really not a factor in power consumption unless you do weird stuff like pump low voltage high current through cable with small cross section, going to even 48VDC@10 amps will lose only ~2% at a distance of 30 meters with crappy 12AWG wires, also appliance power cables are already excluded from wire loss by pure magic 🤔

Testing

Unit tests should probe for majority of the changes, a test is added for wire loss

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Appliance/Power Grid Anything to do with appliances and power grid Code: Tests Measurement, self-control, statistics, balancing. Vehicles Vehicles, parts, mechanics & interactions astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Mar 10, 2023
@irwiss irwiss force-pushed the battery-graph-traversal branch from 2b2be99 to e07aad9 Compare March 10, 2023 14:17
@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Mar 10, 2023
@irwiss irwiss force-pushed the battery-graph-traversal branch 2 times, most recently from 1456fc2 to 43631e4 Compare March 10, 2023 14:42
src/vehicle.cpp Outdated Show resolved Hide resolved
@irwiss irwiss force-pushed the battery-graph-traversal branch from 43631e4 to 22bf4d9 Compare March 10, 2023 16:23
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Mar 10, 2023
@irwiss irwiss force-pushed the battery-graph-traversal branch from 22bf4d9 to 4e6a3f8 Compare March 10, 2023 19:02
@irwiss irwiss marked this pull request as ready for review March 11, 2023 00:38
@kevingranade
Copy link
Member

Hrm.... so how much simplification would we get by removing detailed losses? I'm tempted to say go ahead and swap it out for some nominal value on every transfer.

@irwiss
Copy link
Contributor Author

irwiss commented Mar 11, 2023

We could simplify quite a bit if some constraints are relaxed; if batteries assumed to be hooked in parallel (and so they balance their charge) then this loop and a similar one for discharge could be simplified (and probably made much faster).

From running these loops for each % of battery charge modified - they could be turned into a few flat loops dependent on just number of connected batteries

If wire loss isn't required to be super precise and made on a "best effort" basis then the loss can be turned into an average over connected batteries, together with the change for all batteries to be balanced this means loss calculation is trivial

This will disagree a bit with how it'd work IRL; doesn't simulate internal resistance, voltages, currents and heat of batteries, the time it takes for batteries to balance out as a result of those, voltage fluctuation due to the as a result of the previous etc... But I feel for the players it'll be a more transparent and understandable solution without requiring EE knowledge to understand the game system

@kevingranade
Copy link
Member

All of those sound fine, the important part is that the behaviors it suggests are rational, but that the numbers are precisely correct.

@irwiss irwiss marked this pull request as draft March 12, 2023 17:22
@irwiss irwiss force-pushed the battery-graph-traversal branch from 4e6a3f8 to ad78299 Compare March 13, 2023 22:34
@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Mar 13, 2023
@irwiss irwiss force-pushed the battery-graph-traversal branch from b8b186c to e8a92e4 Compare March 13, 2023 23:23
@irwiss
Copy link
Contributor Author

irwiss commented Mar 13, 2023

Added a couple commits:
When charging/discharging all connected batteries now equalize charge by % (as if they're connected in parallel).
When charging/discharging the power loss is average of all wires ( weighted average by loss_factor * battery_capacity ).
This PR now fixes #64069 by discharging just the amount used by the tool.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Mar 14, 2023
@irwiss irwiss marked this pull request as ready for review March 14, 2023 01:19
@irwiss irwiss force-pushed the battery-graph-traversal branch from e8a92e4 to 0c87ade Compare March 14, 2023 01:20
@github-actions github-actions bot added the <Bugfix> This is a fix for a bug (or closes open issue) label Mar 14, 2023
@irwiss irwiss force-pushed the battery-graph-traversal branch from 0c87ade to e553d80 Compare March 14, 2023 02:56
@irwiss irwiss force-pushed the battery-graph-traversal branch from e553d80 to 52bb641 Compare March 14, 2023 16:19
@irwiss irwiss changed the title Simplify battery graph traversal Refactor wire loss and vehicle battery charging Mar 18, 2023
@irwiss irwiss force-pushed the battery-graph-traversal branch from 52bb641 to 0ff1f92 Compare March 18, 2023 12:28
@irwiss irwiss force-pushed the battery-graph-traversal branch from 0ff1f92 to 52545f2 Compare March 19, 2023 20:11
@kevingranade kevingranade merged commit 7d8dafb into CleverRaven:master Mar 20, 2023
@irwiss irwiss deleted the battery-graph-traversal branch March 20, 2023 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Appliance/Power Grid Anything to do with appliances and power grid astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Charging batteries with water purifier
3 participants