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

Add El Farol Problem in mesa framework under example folder #1057

Closed
wants to merge 31 commits into from

Conversation

danielxu05
Copy link

No description provided.

@rht
Copy link
Contributor

rht commented Dec 5, 2021

The requirements.txt seems to be empty.

self.attend = False

def update_strategies(self):
if (self.model.history[-1] > self.crowdthreshold and self.attend) or (self.model.history[-1] < self.crowdthreshold and self.attend is False):
Copy link
Contributor

Choose a reason for hiding this comment

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

This if condition seems to be the same as the if condition in update_strategies of BarCustomer, if so, it'd be helpful to document that they are the same.

@rht
Copy link
Contributor

rht commented Dec 5, 2021

What is the difference between the IBL and non IBL version? Would be informative to have this in the documentation.

@danielxu05
Copy link
Author

Thanks rht! I have addressed all the comments you have and it should be ready to merge.

@rht
Copy link
Contributor

rht commented Dec 6, 2021

Before this PR is merged, it would be good to squash all the commits into 1 commit, so that the main branch has a clean commit history.

@rht
Copy link
Contributor

rht commented Feb 22, 2022

Bump. I think you should just ignore that particular ipynb from the Codespell CI. You can add the ignore path in

skip: .*bootstrap.*,*.js,.*bootstrap-theme.css.map
.

Also, the .ipynb needs to be linted with Black.

@danielxu05
Copy link
Author

Thanks @rht! I changed it and the commit should be ready to go.

@rht
Copy link
Contributor

rht commented Feb 22, 2022

The diff for python.yml is too big. It seems that your code editor auto formatted it? Also, it should be just the El Farol ipynb, because we do need spell checking for other existing ipynb.

@danielxu05
Copy link
Author

Yes. I think I used the code editor formatted it once. Should I change it back?

@rht
Copy link
Contributor

rht commented Feb 22, 2022

Yeah, so that the diff is minimal, and GitHub doesn't flag the merge conflict.

@rht
Copy link
Contributor

rht commented Feb 22, 2022

I ran the CI. It looks like el_farol.ipynb is not properly Blacked yet. Make sure to use the latest version of Black locally.

@danielxu05
Copy link
Author

I used the lint-black to format el_farol.ipynb multiple times, but it still does not pass the test. Should I also exclude this for lint check ?

@danielxu05
Copy link
Author

I ran the CI. It looks like el_farol.ipynb is not properly Blacked yet. Make sure to use the latest version of Black locally.

I am using the latest ones now.

@rht
Copy link
Contributor

rht commented Feb 22, 2022

Are you on Python 3.9, or 3.11? The CI Black runs on 3.10.

@danielxu05
Copy link
Author

I am on 3.8.3

@danielxu05
Copy link
Author

I used python 3.10 and latest lint black to reformat it, but could not find any changes for the file.

@rht
Copy link
Contributor

rht commented Feb 22, 2022

I'm on Python 3.10, Black 22.1.0, and running Black did indeed reformat the .ipynb. It's mostly removing the trailing "\n".

Another thing I noticed is that the .ipyn is 1.09 MB big. This will bog down the repo for sure. I think you should clear the cells of the computation result.
And, if the cells are cleared, you can reenable Codespell for the El Farol .ipynb, because most of the problems are caused by the image encoding strings.

@rht
Copy link
Contributor

rht commented Feb 26, 2022

@danielxu05 I tried to clean up your PR, tried rebasing it on top of main, but the procedure is too complicated, due to lots of merge commits in this PR. I think, to save time, it is best to make a fresh branch from current Mesa main, and the manually put in the changes and then commit them as one git commit.

I recommend to add this in your ~/.gitconfig:

[pull]
    rebase = true

This way, every git pull will regenerate your commits to be on top of the base branch, i.e. main. No more merge commits.

@rht
Copy link
Contributor

rht commented Nov 25, 2023

Superseded by projectmesa/mesa-examples#69.

@rht rht closed this Nov 25, 2023
@rht
Copy link
Contributor

rht commented Jun 25, 2024

@harshmahesheka this is the original PR that has instance-based learning. My attempt to reproduce the result in the paper: projectmesa/mesa-examples#69 (comment). the RL version should be considered to be sound, as long as the equilibrium is the same as the ABM and IBL version.

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.

3 participants