Add El Farol Problem in mesa framework under example folder

[danielxu05]

[rht]

The requirements.txt seems to be empty.

[rht]

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

[danielxu05]

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

[rht]

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.

[danielxu05]

Sorry for the delay. I have addressed the previous comments. Let me know if there is anything needs to be changed.

[danielxu05]

I am getting build fail. Is there any way to resolve this?

[rht]

This is the error cause: https://github.com/projectmesa/mesa/runs/4572515781?check_suite_focus=true#step:6:132. PyIBL is not installed when running. Would it take a long time to pip install pyibl? There hasn't been any examples that require extra packages installation beyond the "dev" requirement in setup.py, and so this is a new situation.

We can defer the decision on whether to install any packages required by any examples during CI, and just find a way to disable running the example model that uses the pyibl version in tests/test_examples.py.

[danielxu05]

from what I know, PyIBL cannot be installed by using pip. Should I remove pyibl from the requirement file?

[rht]

Yes, you should remove pyibl from requirements.txt, and I assume you will add the install guide of PyIBL in the README. Additionally, you should modify after this line: https://github.com/projectmesa/mesa/blob/cda0495a4c7ca9656cbefd393d9a085b7cbf3367/tests/test_examples.py#L20 to exclude the El Farol example, and also give explanation in the comment why.

[jackiekazil]

This looks good to me... @rht ?

[rht]

This is the remaining TODO: https://github.com/projectmesa/mesa/pull/1057#issuecomment-998398395. Otherwise the tests are broken. The test breaks because the El Farol example still imports from pyibl. The solution is to exclude the El Farol example from test_example.py. See https://github.com/projectmesa/mesa/pull/1057#issuecomment-998398395 for the relevant line to change.

[danielxu05]

I fixed the test file. There are two tests not passed, and both of them indicate some issue in ipynb file. I suspect there are some issues in codespell and lineblack for ipynb files. Let me know what I should do with this. Otherwise, it should be ready to go.

[rht]

If the lint problems are specific to the newly added ipynb in this PR, then you definitely should fix them.

[rht]

@jackiekazil @tpike3 need your approval to run the CI again. It seems to be disabled somehow.

[danielxu05]

The error I got is like this.

Error: ./examples/el_farol/el_farol/el_farol.ipynb:214: te ==> the, be, we, to Error: ./examples/el_farol/el_farol/el_farol.ipynb:214: te ==> the, be, we, to Error: ./examples/el_farol/el_farol/el_farol.ipynb:214: fO ==> of, for Error: ./examples/el_farol/el_farol/el_farol.ipynb:214: fO ==> of, for Error: ./examples/el_farol/el_farol/el_farol.ipynb:214: te ==> the, be, we, to Error: ./examples/el_farol/el_farol/el_farol.ipynb:214: OT ==> TO, OF, OR Error: ./examples/el_farol/el_farol/el_farol.ipynb:247: te ==> the, be, we, to Error: ./examples/el_farol/el_farol/el_farol.ipynb:247: te ==> the, be, we, to Error: ./examples/el_farol/el_farol/el_farol.ipynb:247: Ot ==> To, of, or Error: ./examples/el_farol/el_farol/el_farol.ipynb:247: ND ==> AND, 2ND Error: ./examples/el_farol/el_farol/el_farol.ipynb:247: daA ==> data Error: ./examples/el_farol/el_farol/el_farol.ipynb:333: te ==> the, be, we, to Error: ./examples/el_farol/el_farol/el_farol.ipynb:333: OfO ==> of Error: ./examples/el_farol/el_farol/el_farol.ipynb:333: te ==> the, be, we, to Error: ./examples/el_farol/el_farol/el_farol.ipynb:366: HSi ==> his Error: ./examples/el_farol/el_farol/el_farol.ipynb:366: WHn ==> when Error: ./examples/el_farol/el_farol/el_farol.ipynb:405: thn ==> then Error: ./examples/el_farol/el_farol/el_farol.ipynb:405: eneW ==> new Error: ./examples/el_farol/el_farol/el_farol.ipynb:405: hdA ==> had

I actually did not include any alerted words in the ipynb file. I am not sure whether I am doing something wrong or codespell not working here.

[rht]

Those are probably from the binary files from the plot images. For now, one solution would be to exclude the .ipynb from codespell CI, but this is not ideal. Also, Jupyter notebooks are hard to version in general.

[codecov[bot]]

Codecov Report

Merging #1057 (671d53f) into main (d224397) will increase coverage by 0.35%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1057      +/-   ##
==========================================
+ Coverage   88.92%   89.28%   +0.35%     
==========================================
  Files          19       19              
  Lines        1183     1269      +86     
  Branches      239      259      +20     
==========================================
+ Hits         1052     1133      +81     
- Misses         97      100       +3     
- Partials       34       36       +2     

Impacted Files	Coverage Δ
mesa/space.py	94.92% <100.00%> (-0.99%)	:arrow_down:
mesa/time.py	95.52% <0.00%> (ø)
mesa/__init__.py	100.00% <0.00%> (ø)
mesa/visualization/modules/__init__.py	100.00% <0.00%> (ø)
mesa/batchrunner.py	92.28% <0.00%> (+2.07%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d224397...671d53f. Read the comment docs.

[rht]

Bump. I think you should just ignore that particular ipynb from the Codespell CI. You can add the ignore path in https://github.com/projectmesa/mesa/blob/d346c4879a46052ea75884a980bcbc74fbb27766/.github/workflows/python.yml#L81.

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

[danielxu05]

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

[rht]

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]

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

[rht]

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

[rht]

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]

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]

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]

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

[danielxu05]

I am on 3.8.3

[danielxu05]

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

[rht]

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]

@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.