Skip to content

Conversation

@Regisle
Copy link
Member

@Regisle Regisle commented Jan 3, 2023

This is built onetop of #5507 and doesnt make much sense for base to have

The purpose of this PR is that if sorting by multiple stats and you apply a weight multiplier to one it has little effect, eg fulldps 0.1 ehp 1.0 if my full dps is 100 times bigger itll almost always still just weight and sort by full dps because the change will be 10x bigger after the weight multiplier

by sorting by the percentage change instead it allows them to weight correctly, eg https://www.pathofexile.com/trade/search/Sanctum/LOJYnzGFn with "fulldps 1.0, EHP 0.5" it weights a 1% increased in dps the same as a 2% increase in EHP, rather than almost completely ignoring changes in ehp becouse the absolute difference was so much lower

@QuickStick123 QuickStick123 added the enhancement New feature, calculation, or mod label Jan 3, 2023
Comment on lines 407 to 415
local meanStatDiff = 0
for _, statTable in ipairs(self.calcContext.options.statWeights) do
if statTable.stat == "FullDPS" and not GlobalCache.useFullDPS then
meanStatDiff = meanStatDiff + m_max((output.TotalDPS or 0) / (baseOutput.TotalDPS or 0), m_max((output.TotalDotDPS or 0) / (baseOutput.TotalDotDPS or 0), (output.CombinedDPS or 0) / (baseOutput.CombinedDPS or 0))) * statTable.weightMult
else
meanStatDiff = meanStatDiff + ( output[statTable.stat] or 0 ) / ( baseOutput[statTable.stat] or 0 ) * statTable.weightMult
end
end
meanStatDiff = meanStatDiff * 1000 - (self.calcContext.baseStatValue or 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes some problems when any of the stats taken from baseOutput is 0 - blank (nan) weights in searches or failing to generate searches altogether. Also taking the max of the ratios for full DPS mode leads to disproportionately high weights on very insignificant stats (i.e. increased fire damage on a cold based crit build with a tiny bit of added fire damage, because the relative increase on the tiny ignite is large).

I'm working on top of this branch atm and my current solution is:

function TradeQueryGeneratorClass.WeightedRatioOutputs(baseOutput, newOutput, statWeights)
	local meanStatDiff = 0
	local function ratioModSums(...)
		local baseModSum = 0
		local newModSum = 0
		for _, mod in ipairs({ ... }) do
			baseModSum = baseModSum + baseOutput[mod] or 0
			newModSum = newModSum + newOutput[mod] or 0
		end
		if baseModSum == 0 then
			return newModSum
		else
			return newModSum / baseModSum
		end
	end
	for _, statTable in ipairs(statWeights) do
		if statTable.stat == "FullDPS" and not GlobalCache.useFullDPS then
			meanStatDiff = meanStatDiff + ratioModSums("TotalDPS", "TotalDotDPS", "CombinedDPS") * statTable.weightMult
		else
			meanStatDiff = meanStatDiff + ratioModSums(statTable.stat) * statTable.weightMult
		end
	end
	return meanStatDiff
end

and then

Suggested change
local meanStatDiff = 0
for _, statTable in ipairs(self.calcContext.options.statWeights) do
if statTable.stat == "FullDPS" and not GlobalCache.useFullDPS then
meanStatDiff = meanStatDiff + m_max((output.TotalDPS or 0) / (baseOutput.TotalDPS or 0), m_max((output.TotalDotDPS or 0) / (baseOutput.TotalDotDPS or 0), (output.CombinedDPS or 0) / (baseOutput.CombinedDPS or 0))) * statTable.weightMult
else
meanStatDiff = meanStatDiff + ( output[statTable.stat] or 0 ) / ( baseOutput[statTable.stat] or 0 ) * statTable.weightMult
end
end
meanStatDiff = meanStatDiff * 1000 - (self.calcContext.baseStatValue or 0)
local meanStatDiff = TradeQueryGeneratorClass.WeightedRatioOutputs(baseOutput, output, self.calcContext.options.statWeights) * 1000 - (self.calcContext.baseStatValue or 0)

(and equivalently in a few other places were this logic is duplicated)

@QuickStick123
Copy link
Contributor

Fixes #2144

@LocalIdentity LocalIdentity merged commit 2f4ecd4 into PathOfBuildingCommunity:dev Mar 1, 2023
@Regisle Regisle deleted the percentageSort branch March 1, 2023 07:27
Dullson pushed a commit to Dullson/PathOfBuilding that referenced this pull request Dec 6, 2023
…r than absolute (PathOfBuildingCommunity#5525)

* update item weight and sorting to use percentage change rather than abs

* ratio modweights and dont divide by 0

* change function to local

* change function to local

* make function not local

* fix function call

* fix or 0 math
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature, calculation, or mod

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants