The GUI lies about volcanus overclocks (update: it does not)

[Hermanoid]

No description provided.

[joegnis]

I haven't looked at Volcanus calculation yet, but I suggest adding tests to it when changing values. See tests/test_overclocks.py.

[Hermanoid]

That was a good idea! I ran a dozen tests in 2.5.1 to confirm my test numbers, and the time and parallels I saw have me puzzled. I noticed I had set Volcanus to use modifyGtpp, which I switched to modifyEBF, but the calculations still don't match what I saw in-game.
Here's what I gathered on different coil/voltage combos using the MV steel-from-iron-dust-and-oxygen recipe:

Volcanus Overclock Notes
Steel (base 1000K, 25s, 120 eu/t) :

1. Cupronickel, MV - 108eu/t, 11.40s/ea (no parallel? Half time and then some, but 120% speed would be 10.41s)
2. Kanthal, MV - 103eu/t, 11.40/ea
3. Nichrome, MV - 98eu/t, 11.40/ea
4. Cupronickel, HV - 432eu/t, 11.40/4 (2.9/ea) (half time, 4 parallel)
5. Kanthal, HV - 411eu/t, 11.4/4 (2.9/ea)
6. Nichrome, HV - 390eu/t, 11.4/4 (2.9/ea)
7. Cupronickel, EV - 864eu/t, 11.40/8 (1,4/ea) (half time, 8 parallel)
8. Kanthal, EV - 821eu/t, 11.40/8 (1,4/ea)
9. Nichrome, EV - 780eu/t, 11.40/8 (1,4/ea)
10. Cupronickel, IV - 3456eu/t, 5.70/8 (1.4/s) (quarter time, 8 parallel)
11. Kenthal, IV - 3284eu/t, 5.70/8 (1.4/s)
12. Nichrome, IV - 3120eu/t, 2.85/8 (2.8/s)

@joengis would you recognize this overclocking pattern?

[Hermanoid]

Relevant hunk of code:
https://github.com/GTNewHorizons/GTplusplus/blob/38f38a991e433f6eff30476b87a71eeadee228ce/src/main/java/gtPlusPlus/xmod/gregtech/common/tileentities/machines/multi/processing/advanced/GregtechMetaTileEntity_Adv_EBF.java#L235

And https://github.com/GTNewHorizons/GTplusplus/blob/master/src/main/java/gtPlusPlus/xmod/gregtech/api/metatileentity/implementations/base/GregtechMeta_MultiBlockBase.java#L99

I will revisit this in a few days.

[Hermanoid]

Turns out the pattern matches Utupu, although I had a very informative time figuring out the OC calculations through experimentation.

[OrderedSet86]

There were issues with GT++ math before, should be fixed now. Try newest gtnh-flow code and see if it still has issues

[Hermanoid]

I re-ran my volcanus tests with the gtpp logic and it is missing ebf heat bonuses and perfect overclocks. I do think it needs the Utupu logic.

[Hermanoid]

@OrderedSet86, Any news? I'm using Volcanus quite a bit and it would be handy if this were on main.

[joegnis]

"HSS-G" should be lowercase since mod_recipe directly modifies value without taking care of cases. Right now, the cases are handled by Recipe's __init__ which is skipped when we use mod_recipe. I should've made it handle cases better.

[joegnis]

Thank you for writing those tests! I personally verified those tests in game and they are correct. So algorithm is probably alright, but I am not sure if it affects Utupu's calculation, since it changes modifyUtupu.

[Hermanoid]

@joegnis yeah it's a crappy way of handling the overlap in functionality. modifyUtupu works normally when you don't specify "override_max_parallel", but there could be an argument for either a clearer way of writing this (maybe both functions call a generic "modifyWithHeatBonus". There could also be an argument to rework the whole overclock system to somewhat mirror the GT Java code, which (as I understand it) is arranged into steps that are turned on/off and configured. The usual format for everything except turbines and odd exceptions is something like apply Gtpp power bonuses, apply parallels (per tier, max, or fixed), calculate overclocks (perfect, regular, goofy), apply heat bonuses. I would argue there aren't enough machines to warrant such a rework,

[Hermanoid]

Now that I think about it, a system could handle most cases with a beefed-up yaml format. This would need some thinking to handle the many odd cases, but something like this:

volcanus:
  speed_boost: 1.2  # Other machines could just exclude these if they don't have it
  eu_discount: 0.8
  parallel_max: 8  # Others could use parallel_per_tier or parallel_fixed. Chem plant could be "from_casings"
  overclock: ebf  # Others could use regular, perfect, goofy, etc

[Hermanoid]

Such a rework would be a good chance to implement tick rounding.

[joegnis]

Good news! I am actually back working on a rework right now. I am trying to do the "mirror Java code" way you mentioned.

Now that I think about it, a system could handle most cases with a beefed-up yaml format. This would need some thinking to handle the many odd cases, but something like this:

volcanus:
  speed_boost: 1.2  # Other machines could just exclude these if they don't have it
  eu_discount: 0.8
  parallel_max: 8  # Others could use parallel_per_tier or parallel_fixed. Chem plant could be "from_casings"
  overclock: ebf  # Others could use regular, perfect, goofy, etc

IMO, this seems simpler but if you look at Java code, since there are so many types of machines, there are other cases where an argument of the calculator is not simply a value but a Java object, so a yaml file is not enough. It may be possible to fit all arguments of a machine's calculator to a Python object, but I will see when I work on the rework.
Right now, I have working OverclockCalculator implementation, but I still need a working ParallelHelper to support GT++ machines.
And thank you for mentioning the tick rounding issue, I will test if that can be solved along the way.

[Hermanoid]

@joegnis how goes this rework? @OrderedSet86 would it be possible to have this implementation merged For Now™?

[joegnis]

@joegnis how goes this rework? @OrderedSet86 would it be possible to have this implementation merged For Now™?

Soon™. See #53

[OrderedSet86]

@Hermanoid Please use git rebase in the future, it avoids polluting the commit log with unnecessary master merge commits. It is fine for this PR though

Yes, this PR is more or less fine to merge for now (with the exception of the hss-g capitalization which I have fixed). Thank you for adding tests.

I'm not sure when this happened, but the sanity test seems to be failing more projects now - this is concerning and should be looked into

[OrderedSet86]

Good news! I am actually back working on a rework right now. I am trying to do the "mirror Java code" way you mentioned.

Now that I think about it, a system could handle most cases with a beefed-up yaml format. This would need some thinking to handle the many odd cases, but something like this:

volcanus:
  speed_boost: 1.2  # Other machines could just exclude these if they don't have it
  eu_discount: 0.8
  parallel_max: 8  # Others could use parallel_per_tier or parallel_fixed. Chem plant could be "from_casings"
  overclock: ebf  # Others could use regular, perfect, goofy, etc

IMO, this seems simpler but if you look at Java code, since there are so many types of machines, there are other cases where an argument of the calculator is not simply a value but a Java object, so a yaml file is not enough. It may be possible to fit all arguments of a machine's calculator to a Python object, but I will see when I work on the rework. Right now, I have working OverclockCalculator implementation, but I still need a working ParallelHelper to support GT++ machines. And thank you for mentioning the tick rounding issue, I will test if that can be solved along the way.

This does seem promising, and we have the advantage of knowing the end user already has a functioning Java, otherwise GTNH wouldn't run. Maybe we can look it up the Java from the user's prism/curse/multimc/polymc config? Just a thought, not sure how viable this would be

I don't think this is part of your implementation, but I would caution that I want to avoid requiring a running GTNH instance unless there is a very good reason (like reading the recipe map from the running game).

[OrderedSet86]

I'm not sure when this happened, but the sanity test seems to be failing more projects now - this is concerning and should be looked into

Looks like it is name canonization issues, I broke it a few months ago #52