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

Added homophily ratio in basic schelling example #2520

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

vbv-shm
Copy link

@vbv-shm vbv-shm commented Nov 26, 2024

Changed homophily comparing logic as per discussion in #2515.
Now homophily_ratio is checked for agents to be happy. homophily_ratio is similar neighboring agents divided by the total number of neighbors.

@vbv-shm vbv-shm changed the title Fix #2515 as per discussions in issues. Added homophily ratio in in basic schelling example #2515 Nov 27, 2024
@vbv-shm vbv-shm changed the title Added homophily ratio in in basic schelling example #2515 Added homophily ratio in basic schelling example Nov 27, 2024
@@ -13,7 +13,7 @@ def __init__(
width: int = 40,
density: float = 0.8,
minority_pc: float = 0.5,
homophily: int = 3,
homophily_ratio: float = 0.3,
Copy link
Member

Choose a reason for hiding this comment

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

why 0.3? It would make more sense to go to 3/8 (8 is the neighborhood size of a Moore grid with a radius of 1 and all fields occupied.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for reviewing. I will change 0.3 to 3/8 (0.375).

Copy link
Author

Choose a reason for hiding this comment

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

@quaquel The step size of the slider is 0.1. So the ratio can either be 0.3 or 0.4.
If the ratio is 0.3, agents with 3 similar neighbors and 8 total neighbors will be happy, as 3/8 = 0.375.
And if the ratio is 0.4, agents with 3 similar neighbors and 8 total neighbors won't be happy.
So should I change 0.3 to 0.4?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can change the step size to 0.125 as well?

@EwoutH EwoutH added the example Changes the examples or adds to them. label Nov 27, 2024
@EwoutH
Copy link
Member

EwoutH commented Nov 27, 2024

Thanks for the PR! Could you update your PR description to contain all the necessary information to understand this change on its own? You can use this template if you want.

Edit: I saw there's also this PR open, which also modifies schelling:

Are there any conflicts between them? I would tend to let #2518 go first, since it was opened earlier. (or did we assign someone to this specific issue?)

@vbv-shm vbv-shm closed this Nov 27, 2024
@vbv-shm vbv-shm reopened this Nov 27, 2024
@vbv-shm
Copy link
Author

vbv-shm commented Nov 27, 2024

@EwoutH Thank you for the reply.
I have understood that #2815 will be given priority because it was opened first. Still, can I work on this PR just in case #2815 doesn't work out? Or is this PR required to be closed?
From now on, I will be careful not to open PR for some issue if PR is already opened.

@quaquel
Copy link
Member

quaquel commented Nov 27, 2024

I have provided feedback in #2518, if that is not addressed in the coming days and there is no further response, I am fine with merging this instead.

@vbv-shm
Copy link
Author

vbv-shm commented Nov 28, 2024

@quaquel Thank you. I understand.

@EwoutH EwoutH added trigger-benchmarks Special label that triggers the benchmarking CI and removed trigger-benchmarks Special label that triggers the benchmarking CI labels Nov 29, 2024
@vbv-shm
Copy link
Author

vbv-shm commented Dec 6, 2024

@EwoutH May you please run the benchmark tests?

@vbv-shm
Copy link
Author

vbv-shm commented Dec 9, 2024

@quaquel @EwoutH Previously the benchmark tests were failing because I changed homophily to homophily_ratio in the Schelling example. homophily was provided by the configuration file while the model now takes homophily_ratio as input. In order to fix this, I changed homophily to homophily_ratio inside the configuration.
I am curious if my fix is right or not. Could you please run the benchmark tests so that changes can be checked?

@EwoutH EwoutH added trigger-benchmarks Special label that triggers the benchmarking CI and removed trigger-benchmarks Special label that triggers the benchmarking CI labels Dec 9, 2024
Copy link

github-actions bot commented Dec 9, 2024

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 -2.4% [-3.0%, -1.7%] 🔵 -0.8% [-1.0%, -0.6%]
BoltzmannWealth large 🔵 -1.3% [-2.5%, -0.1%] 🔵 +0.1% [-0.7%, +0.9%]
Schelling small 🔵 -1.0% [-1.4%, -0.7%] 🟢 -12.7% [-13.3%, -12.1%]
Schelling large 🔵 -0.2% [-0.7%, +0.4%] 🔴 +87.2% [+85.6%, +88.6%]
WolfSheep small 🔵 -2.6% [-3.3%, -1.9%] 🟢 -6.8% [-7.2%, -6.3%]
WolfSheep large 🟢 -4.8% [-5.5%, -3.9%] 🟢 -8.3% [-9.8%, -6.8%]
BoidFlockers small 🔵 -1.5% [-2.2%, -0.9%] 🔵 +2.3% [+1.5%, +3.1%]
BoidFlockers large 🔵 -2.8% [-3.9%, -1.6%] 🔵 +1.4% [+0.7%, +2.2%]

@vbv-shm
Copy link
Author

vbv-shm commented Dec 10, 2024

Looks like benchmarks ran, but performance is not up to the mark. So changing homophily to homophily_ratio inside the configuration worked.

@EwoutH
Copy link
Member

EwoutH commented Dec 10, 2024

Thanks for your work. We’re currently waiting on #2518 to be performance validated, one that’s done we can continue with rebasing this PR and checking it’s performance properly.

It’s great it now runs without problem!

@vbv-shm
Copy link
Author

vbv-shm commented Dec 11, 2024

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
example Changes the examples or adds to them.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants