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

estimate matching #1009

Merged
merged 11 commits into from
May 31, 2023
Merged

estimate matching #1009

merged 11 commits into from
May 31, 2023

Conversation

aminlatifi
Copy link
Member

@aminlatifi aminlatifi commented May 25, 2023

@aminlatifi aminlatifi marked this pull request as draft May 25, 2023 13:17
@CarlosQ96
Copy link
Collaborator

CarlosQ96 commented May 31, 2023

Will Merge it to upgrade the main branch of Mohammad this way we can unblock frontend in dev server
I think the calculationss are correct I added new queries and helpers. Will add tests in main branch. @mohammadranjbarz

@CarlosQ96 CarlosQ96 marked this pull request as ready for review May 31, 2023 05:09
@CarlosQ96
Copy link
Collaborator

HOWEVER, following AMin's idea with the PR a query was made with the different parts of the formula. I think to calculate this frontend should do it, can be expensive for backend:
image

@CarlosQ96
Copy link
Collaborator

Or we can make a method receiving multiples values and return them all at once.

@CarlosQ96 CarlosQ96 merged commit fe1772a into f_980_implement_qfRound May 31, 2023
@aminlatifi
Copy link
Member Author

@CarlosQ96 Thanks for finishing this PR, but there is a point. My idea was to use getProjectDonationsSqrtRootSum instead of getQfRoundTotalProjectsDonationsSumExcludingProjectById which you has created.
First, that query is cheaper to run, and the same result can be achieved in the frontend. What do you think?
Besides, you have added to methods which would clutter the code.

@CarlosQ96
Copy link
Collaborator

CarlosQ96 commented Jun 4, 2023

Yeah lets optimize now, I think the first step was to unblock cherik and ramin since we had them there waiting. I think it's the correct idea to do the calculations in the frontend. I thought that would be your idea.

However!!!!

The reason I made getQfRoundTotalProjectsDonationsSumExcludingProjectById its because if they are going to simulate the added estimated matching based on 9 dai, 10 dai , 20 dai like in the following picture:
image

They have to recalculate the total, this total, can't include the current square-root of the project since it changed. They have to add the new squareroot to the sum and to the total.
Then subtract NewEstimateMatching - CurrentEstimatedMatching = added estimated value per donation.

That's the way I saw to calculate the image above. Or is there an easier way?

@CarlosQ96
Copy link
Collaborator

What do you think @aminlatifi ?

@aminlatifi
Copy link
Member Author

e way I saw to calculate the image above. Or is there an easier wa

I think there is an easier way, but I didn't explain it here, sorry for that.
The front-end can easily calculate the square root of the project and reduce it from the total value! Then to obtain the new value do the calculation again with computing the modified square root of the project donations and and apply the different to the total sum too. @CarlosQ96 Does it make sense?

const projectSquareRootSquared = Math.Power(projectSquareRoot,2);
const totalExceptProject = TotalSum -projectSquareRootSquared ;
const newProjectSquareRoot = projectSquareRoot + Math.sqrt(newDonationValue);
const newProjectSquareRootSquared = Math.power(newProjectSquareRoot, 2)
const newTotal  = totalExceptProject + newProjectSquareRootSquared
....

@CarlosQ96
Copy link
Collaborator

Yeah thanks, I overlooked that way. I agree.

@aminlatifi aminlatifi deleted the f_988_estimate_matching branch August 28, 2023 07:35
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