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

fix: expectedContributionPoints #143

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pegahcarter
Copy link
Contributor

@Jabyl please confirm that _calcExpectedContributionPoints() is correct. This function change means that in calculating performance of a member for a previous period, the amount of contribution points the member should earn is compared to the cumulative active (aka outstanding) and given contribution points.

@Jabyl
Copy link
Contributor

Jabyl commented Dec 11, 2024

Hi @pegahcarter, no - that's not how it works.

For what I understand, based on the way you designed the Contribution system, the Total Contribution Points (tCP) would be the sum of all Given Contribution Points of all Members in a period, plus the "outstanding Contribution Points" at the time of taking the snapshot.

Please note that in your initial message, you seem to be using "Performance" in stead of "Participation Score" - Please refer to our public documentation only, and let's stick to our conventions.

  1. we do not calculate anything related to a "previous" period.
  2. Performance is calculated simply as the ratio GCp / ECp, where GCp = Given Contribution Points (the points acquired in a period by completing tasks/contributions), and ECp = Expected Contribution Points (the points you're expected to complete in a period).
  3. Participation Score (PS) starts from 100, for each new Member in a Hub - and it's then updated after each period, by multiplying the PS of a Member in the previous period by the Performance of that member in the new period. It uses this formula.
  4. Expected Contribution Points (ECp) are calculated multiplying Sum of all Contribution Points created (through Tasks/Contributions) in a Hub during a given period by the fractional Commitment Level of a Member (fCL).
  5. The fractional Commitment Level of a member is calculated by dividing a Member's Commitment Level (CL), which can be between 1 and 10) by the sum of all the CLs of all members. Refer to this formula
  6. Example on how to calculate the total Commitment Level (tCL) of a Hub in a given period: if in period 3 in a Hub there are 80 members, whose average CL is 8, the total Commitment Level of that Hub in Period 3 will be 640 (8+8+8+...8 for 80 times).

@pegahcarter
Copy link
Contributor Author

pegahcarter commented Dec 12, 2024

I appreciate your well-written and thought out response. You have a deep understanding of the protocol and it's very helpful in fine-tuning parameters and formulae. Regarding points raised:

  1. we do not calculate anything related to a "previous" period.

When period X passes and we're in period X+1, we do need to calculate performance in period X when writing previous performance to storage. We have a helper view of calcPerformanceInPeriod(commitmentLevel, sumPointsGiven, period) exactly for this, which works for the current period as well as any previous period.

  1. Performance is calculated simply as the ratio GCp / ECp, where GCp = Given Contribution Points (the points acquired in a period by completing tasks/contributions), and ECp = Expected Contribution Points (the points you're expected to complete in a period).

Correct. Code here:

uint128 performance = (1e18 * sumPointsGiven) / expectedContributionPoints;

  1. Participation Score (PS) starts from 100, for each new Member in a Hub - and it's then updated after each period, by multiplying the PS of a Member in the previous period by the Performance of that member in the new period. It uses this formula.

Correct. It's a common practice across Solidity to use 10*18 to reduce rounding, which I've used as equivalent to 100. Code here:

if (performance > 1e18) {
// exceeded expectations: raise MemberActivity participationScore
delta = performance - 1e18;
factor = constraintFactor();
if (delta > factor) delta = factor;
participationScore = (previousScore * (1e18 + delta)) / delta;
} else {
// underperformed: lower MemberActivity participationScore
delta = 1e18 - performance;
factor = penaltyFactor();
if (delta > factor) delta = factor;
participationScore = (previousScore * (1e18 - delta)) / delta;
}

  1. Expected Contribution Points (ECp) are calculated multiplying Sum of all Contribution Points created (through Tasks/Contributions) in a Hub during a given period by the fractional Commitment Level of a Member (fCL).

This is calculated here:

function _calcExpectedContributionPoints(uint32 commitmentLevel, uint32 period) internal view returns (uint128) {
return
(fractionalCommitment({commitmentLevel: commitmentLevel, period: period}) *
ITaskManager(taskManager()).getSumPointsActive(period)) / 1e18;
}

And this is where I raised the question in this PR. Before this PR, ECp corresponded solely to the amount of contribution points that can be actively given. This means that if the amount of contribution points to be given goes down because contributions are given to members, ECp would change accordingly. After this PR, ECp would remain the same after points are given as it includes points given.

What it sounds like you're saying is that ECp should be calculated by the amount of contribution points created within a period. Which means that if active contribution points are 100 from previous periods but created contribution points in the current period are 50, we would only look at the 50.

From the docs:

"TCP: Total Contributions Points in a Community. Sum of all contributions points available in a community in a given period. "

Ultimately, this definition is unclear for me as I don't know if "points available" includes previous periods. After this PR, the definition would be:
"TCP: Sum of all outstanding contribution points to be given and contribution points given at the end of a period."

What do you think?

  1. The fractional Commitment Level of a member is calculated by dividing a Member's Commitment Level (CL), which can be between 1 and 10) by the sum of all the CLs of all members. Refer to this formula

Correct:

function fractionalCommitment(uint32 commitmentLevel, uint32 period) public view returns (uint128) {
return (1e18 * uint128(commitmentLevel)) / IMembership(membership()).getSumCommitmentLevel(period);
}

  1. Example on how to calculate the total Commitment Level (tCL) of a Hub in a given period: if in period 3 in a Hub there are 80 members, whose average CL is 8, the total Commitment Level of that Hub in Period 3 will be 640 (8+8+8+...8 for 80 times).

Agreed.

@Jabyl
Copy link
Contributor

Jabyl commented Dec 12, 2024

Hi @pegahcarter, thanks for the follow-up.

Getting to your points:

What it sounds like you're saying is that ECp should be calculated by the amount of contribution points created within a period. Which means that if active contribution points are 100 from previous periods but created contribution points in the current period are 50, we would only look at the 50.

No, I don't mean the points "created" - but the points that we could define "active."
This includes both the points created through tasks/contributions in the ongoing period, as well as the points inherited from a previous period. This means that we need to calculate it dynamically as: total Contribution Points + Outstanding Points.
The reason is simple:

  • total Contribution Points are the points given by the Members by completing tasks, that means: it's a growing-only amount.
  • on the other hand, Outstanding Points tend to decrease (after completing Tasks/Contributions, Members "receive" points, and these points are no longer available. Therefore, the Outstanding Points decrease.) I say that they "tend" to decrease rather than they always decrease, because it is possible to create new tasks/contributions till the end of a given period, and any time a new task is created, the "weight" (= amount of points / worth) of that task increases the Outstanding Points count.

Based on the above, the only way to calculate continuously the amount of active/available points in a Hub in a given Period is tCP (the total amount of Contribution Points given by all Members of a Hub in a given period) + Outstanding Points.

From the docs: "TCP: Total Contributions Points in a Community. Sum of all contributions points available in a community in a given period. " Ultimately, this definition is unclear for me as I don't know if "points available" includes previous periods.

Here "in a given period" is key. What I mean to say in the definition is that we calculate all CP-based parameters both for Individuals (= the Members: eCP, gCP) and for Hub (= tCP) on a period basis. The Expected Contribution Points of a Member are calculated on a period-basis, and your required eCP as a member in a specific PeriodID will never (or very rarely) be the same of any other past or future period.

After this PR, the definition would be:
"TCP: Sum of all outstanding contribution points to be given and contribution points given within a period." What do you think?

I think it's correct, let's use the formula I created above to make sure we are on the same page:
tgCP (total given contribution points - in the ongoing period) + Outstanding Contribution Points (in the ongoing period)

Overall, this is an important conversation - I'm glad we are getting there. I understand it's difficult in general, so thank you for your patience and the hard work.

@pegahcarter
Copy link
Contributor Author

Okay, sounds good! To summarize:

ECp is the sum on given and outstanding contribution points of the ongoing period.

This works to keep the current period ECp consistent even as points flow from "active" to "given". Should ECp of a previous period also take this summation of given and outstanding contribution points?

total Contribution Points are the points given by the Members by completing tasks, that means: it's a growing-only amount.

We should further define TCP here because ECp uses TCP, and if we're using given and active contribution points for ECp, TCP is not guaranteed to be a growing-only amount.

Additionally, we should add ACP "Active Contribution Points" to the Core Parameters docs.

My impression from this discussion is we can formally define TCP as GCP + ACP for a given period.

@Jabyl
Copy link
Contributor

Jabyl commented Dec 12, 2024

Hey Carter:

ECp is the sum on given and outstanding contribution points of the ongoing period.

This doesn't sound right. eCP should always be calculated as tCP(1) * fCL (fractional Commitment Level).
(1): based on the conventions below, this could be aCP to increase clarity.

Should ECp of a previous period also take this summation of given and outstanding contribution points?

eCP of past periods is no longer relevant, and should not be recalculated or updated. We should just record each Member's previous eCPs at the time of the final calculation (the final "snapshot" we take before moving from a period to the next) so that they can be indexed and queried by us or other 3rd-parties for statistical/analytical reasons. Past eCPs, though, should not be considered for any of the "ongoing period" calculations, and each period's eCP of a Member is independent from any previous one.

total Contribution Points are the points given by the Members by completing tasks, that means: it's a growing-only amount. We should further define TCP here because ECp uses TCP, and if we're using given and active contribution points for ECp, TCP is not guaranteed to be a growing-only amount. Additionally, we should add ACP "Active Contribution Points" to the Core Parameters docs.

Good point, let's call this "growing amount" (the "total given contributions of all members") as \sum tgCP.
Whereas we use aCP for the calculation of the eCP of each member*.
*: I'll also clean up the Docs, so that the parameters follow more closely the contracts implementation.

My impression from this discussion is we can formally define TCP as GCP + ACP for a given period.

Conceptually is correct, but based on the conventions we used above, I believe it should be rather:
aCP = (\sum tgCP) + oCP
where oCP: the Outstanding Contribution Points in an ongoing period.

@pegahcarter
Copy link
Contributor Author

You're right. eCP should always be calculated as tCP(1) * fCL (fractional Commitment Level). The code reflects this as well, I said it wrong.

To summarize

  • we should define aCP as "Active Contribution points", as the cumulative value of "\sum tgCP" (what should this be called in Solidity?) and "oCP" ("Outstanding Contribution points").
  • Past eCP is "frozen" as it does not does change once a period has ended and is not relevant to current eCP.
  • Member eCP is based on their commitment level and is separate from one member to another.
  • Member performance and participation score is calculated based on the "frozen" eCP value.

@pegahcarter
Copy link
Contributor Author

@Jabyl can you please clarify so I know how to write the variable names?

Right now the contracts use:

  • sumPointsActive: equivalent to "outstanding contribution points" (oCP)
  • sumPointsGiven: equivalent to "sum \tgCP"
  • sumPeriodPoints: sumPointsActive + sumPointsGiven

Are the current variable names sufficient? Please let me know what they should be and I'll update this PR / merge.

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

Successfully merging this pull request may close these issues.

2 participants