-
Notifications
You must be signed in to change notification settings - Fork 409
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
Updated RAH to version 2. Thank you @ccplarrikin #688
Conversation
All right. So I completely rewrote the functionality. Instead of using multiple tuples, using a single one. Little simpler and easier (and probably ever so slightly faster). Then what we do is rather simple.
Lather, rinse, and repeat. If we run out of resists to steal, we stop. If the resist that has taken the least amount of damage is at 0 over 15 loops, we stop. Most of the time it'll be done in 5 loops. Testing against various profiles shows that it's taking about 180-250 MS to run, so nice and quick. We will need to test against more complex damage profiles to see if that changes any. |
Paging @MrNukealizer and @blitzmann for testing. |
As a reminder to myself (and a heads up to others), because the merge from Pyfa/Master broke, it created changes for all the diffs, which is why there's 100+ files modified. Once we get the functionality and bugs worked out, I'll prune this branch and submit a new (clean) PR with just the changes we want to the RAH. |
to better show how the multiple damage profile handling works.
Removed as this was inaccurate. |
And now updates the armor and modified damage done per cycle
but still not calculating correctly. :(
Okay. Updates made, looks (to me) like it's calculating properly. It probably won't fully calculate properly with the DCU...but... So, some benchmarks. All tests done with restarting Pyfa and loading the fit for the first time. Vengeance with 3 boosters, multiple projected, and damage profile with 3 types:
Vengeance with multiple projected, and damage profile with 3 types:
Vengeance with damage profile with 3 types:
Gnosis with uniform resists and uniform damage profile with 3 types: (hits cycle limit)
The cycle limit is 15 cycles after one of the resists hits 0. We can probably shorten that up. The start/end times are a wee bit deceptive, even for the Gnosis the actual RAH cycling time is only about 100ms. I don't think we can make it much faster if we want to keep it fully adaptive... |
That could definitely be a more efficient way to run it, but the current code gives incorrect results. You seem to be hard coding some values that should be fetched from the module attributes. For example:
assumes the starting resonances are all 0.85, but what if that's changed or a T2 version is added? I got around that by grabbing the resonances before changing anything and using that as a base.
This seems to be the source of inaccuracy. I don't know where you got 3% from, but it's the After changing that to 6% the algorithm matches most of my tests. The two exceptions were when two or more types of damage did exactly the same amount, i.e. attacking a Gnosis with a Gecko or a T1 Gallente ship with Null and explosive drones, and another scenario with a much worse result. To fix the even damage into even resistances problem, the initial tuple order should be sorted alphabetically: EM, Explosive, Kinetic, Thermal instead of EM, Thermal, Kinetic, Explosive. That causes the sorting to give equal results the same order the game uses. Now for the test that went horribly wrong:
with a damage profile of 1/2/2/1 (sentry guns in lowsec) will never cause any of the RAH resistances to reach 0, and thus the loop will never stop. Aside from those details there's one other accuracy issue. If the RAH never stops changing the code will break out of the loop and give the result from the last cycle. When the RAH loops like that, no single cycle accurately represents the effective resistances over the course of the loop. The effective resistances will be the average of all the cycles in the loop. That's why my code saves the results of previous cycles, identifies a loop, then gives the average of all the cycles in the loop. One last thing: it's more of a design choice than an accuracy issue, but I still think the code should handle uniform damage rather than staying at the default resistances. There's the specific example of seeing how a ship fares against Geckos, but more than that people use uniform damage to show how the ship handles a "general" situation where they get shot by any and all types of damage. In such a situation the starting resistances of the RAH are meaningless whereas the profile it settles on can give a good idea of the strengths and weaknesses of the armor and how well the RAH will compensate. |
I still don't understand why my code has performance problems when you run it but this doesn't. When I test the timing my code takes about half as long as this does when the RAH loops, and about the same time when the RAH stops on one profile. Both versions also seem to run 5-20 times faster in my tests than in yours, which seems like more than hardware differences should account for. |
If CCP adds a T2 version or changes skills related to this or anything, really, it'll probably go to hell in a hand basket. We can store the original resistance and use that, that's not a big deal.
Easily changed. Now that I'm looking at it, there's even a value for how much gets transferred.
I'll poke around and see how to sort multiple columns. If it easy, then I'll add it. It's a pretty extreme edge case to be honest, as the vast majority of scenarios won't have even damage.
First of all, never ending RAH loops are going to be an edge case. There's no real great way to handle them, as by it's very nature_anything_ we display will be wrong. Loops inside loops can get expensive, code wise, which is why I avoided that as a solution. I understand that grabbing the last one is fairly arbitrary. Hmm, a thought occurs to me that might be a solution. We can lower how much it transfers by .01 each loop, which should cause it to basically average out without having to store and loop through the last few resist profiles. I'll test that.
I'm not super thrilled with the performance of this either when you hit the max number of runs. But as soon as we open the door to something outside of the simple 30/30 resists, performance takes a huge hit. I can't really explain why my environment runs slower than yours, but I CAN safely say that my environment definitely won't be the slowest potential environment that Pyfa runs on. So it's only downhill from here. I did submit a PR to potentially break the refresh calcs out, which would dramatically help with times (not so much the individual RAH times as the overall times). I will also rerun your version and do some benchmarking, once I update this version.
Noted and I half agree with you. We could key off the resist profile name, but I really dislike hard coding that. There isn't a good answer here, because to the end user any even profile looks the exact same (all 25 percent). |
So one edge case that this setup covers that others might not...
I actually didn't plan to cover this, but glad it doesn't break out as soon as the resist are fully allocated.
Done.
Done, easy peasy.
Done and done. This actually works very nicely. We could actually go lower than .01, but then (potentially, if we go too low) it shows in the fitting window weird due to rounding issues. |
Just in case CCP mucks with values, it also will populate the affected by tab in a fashion more similar to how the multiple damage type profiles work.
All righty, some profiling. Gnosis with nothing but a RAH, damage profile of v2 code (updated)
v3 code
So a few things. One, the times are pretty close to the same (4-5 ms difference depending on where you look), but v2 loops 18 times while v3 loops 6 times. Additionally, v3 should fall into an infinite loop. It doesn't, which means that there's something not quite right there.
You can see that the resist end results aren't quite what they should be. We can probably knock off a few loops off v2 in the never ending cycle scenario (which would bring down the total execution time some), but 15 seems a reasonable compromise. Additionally there's more debug logging on v2, which slows things down a bit further. Using a single tuple and eliminating a lot of the extra loops seems to have made a significant difference in execution time. I'm still not thrilled with it, but I don't see any ways of further improving that without cutting functionality. It bugs me a bunch that this doesn't handle stacking penalties very well, but my first attempt at a rewrite tried updating the ship stats to handle it (which it did!), the problem was that there's currently no clean way of updating an existing modification or removing it. You can remove all modifications (which puts the ship back to base stats), and you can soft set the stat, but stacking penalties kick in multiple times and the values go off the rails. This is probably as close as we're going to get. At this point in time, unless someone else ( @MrNukealizer or @blitzmann ) see further improvements that can be made to the RAH code to speed it up |
Now will handle cases where you could enter an infinite loop. Should be impossible to cycle forever now, will break out after 100 total loops or 5 loops after the resist transfer amount is lowered to .01. Another advantage is that it can start reducing the resist transfer amount in fewer loops, so for many fits will end up taking fewer loops to run.
Closing this as it has been replaced with #737 |
For wall of text regarding the history of this read #680