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

Importable examples #2381

Merged
merged 4 commits into from
Oct 17, 2024
Merged

Importable examples #2381

merged 4 commits into from
Oct 17, 2024

Conversation

Corvince
Copy link
Contributor

@Corvince Corvince commented Oct 16, 2024

This makes the basic examples importable. You can now do from mesa.examples.basic import BoltzmannWealthModel.

Tried a few hours to convince hatch to link examples into mesa. Got it working eventually, but it broke editable installs. Since editable installs are the best feature since bread and butter the error there pointed me towards simply specifiing __path__ in a stub example module.

This PR only works on basic examples, because the advanced ones haven't been updated yet to a better standard.

Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +3.3% [+2.1%, +4.5%] 🔵 +0.8% [+0.6%, +0.9%]
BoltzmannWealth large 🔵 +1.1% [+0.1%, +2.2%] 🔵 +0.7% [-0.3%, +1.7%]
Schelling small 🔵 +0.2% [-0.3%, +0.7%] 🔵 -0.3% [-0.9%, +0.2%]
Schelling large 🔵 +0.8% [-0.4%, +2.1%] 🔵 -2.0% [-3.9%, +0.1%]
WolfSheep small 🔵 +0.0% [-0.8%, +0.9%] 🔵 -0.6% [-1.2%, +0.0%]
WolfSheep large 🔵 +0.8% [-1.5%, +2.9%] 🔵 -0.6% [-2.6%, +1.1%]
BoidFlockers small 🔵 +1.9% [+0.9%, +2.8%] 🔵 -0.4% [-1.1%, +0.3%]
BoidFlockers large 🔵 +0.9% [+0.2%, +1.7%] 🔵 -0.2% [-0.8%, +0.4%]

@EwoutH
Copy link
Member

EwoutH commented Oct 17, 2024

Awesome, thanks for working on this! Let me know when you need a review or something else.

@Corvince Corvince force-pushed the importable-examples branch from aff5ea7 to 706f03d Compare October 17, 2024 14:54
@Corvince
Copy link
Contributor Author

@EwoutH (and others) this is now ready for review! For now only the basic examples are covered, until the others are updated.

@Corvince Corvince marked this pull request as ready for review October 17, 2024 15:01
Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +1.9% [+1.0%, +3.0%] 🔵 -0.0% [-0.2%, +0.2%]
BoltzmannWealth large 🔵 +1.0% [+0.1%, +1.9%] 🔵 +2.9% [+1.4%, +4.4%]
Schelling small 🔵 +0.8% [+0.5%, +1.2%] 🔵 +1.0% [+0.7%, +1.4%]
Schelling large 🔵 -0.2% [-1.4%, +0.9%] 🔵 +1.0% [-0.7%, +2.6%]
WolfSheep small 🔵 +1.7% [+1.1%, +2.3%] 🔵 -0.7% [-1.1%, -0.4%]
WolfSheep large 🔵 +2.2% [+0.7%, +3.6%] 🔵 +4.2% [+1.4%, +7.0%]
BoidFlockers small 🔵 +0.0% [-0.4%, +0.5%] 🔵 +0.6% [-0.1%, +1.3%]
BoidFlockers large 🔵 -0.5% [-1.2%, +0.1%] 🔵 -0.7% [-1.6%, +0.2%]

@Corvince
Copy link
Contributor Author

Corvince commented Oct 17, 2024

I have now fixed the visualisation tutorial as well. I originally thought we have to fix that in mesa-examples, but we dont so I included it here

/edit *tried to fix it

@Corvince
Copy link
Contributor Author

@quaquel Do you have any idea why this might be failing on RTD? Is rtd doing something special or building mesa differently?

@EwoutH
Copy link
Member

EwoutH commented Oct 17, 2024

pip install --quiet --upgrade --pre mesa

that doesn’t look like the tutorial uses the version from this PR, could that be the culprit?

@Corvince
Copy link
Contributor Author

Ah, yes, of course. I thought there is a problem with the version on RTD, but the error is of course inside the notebook. Thanks!

So theoretically this will solve itself with the next pre-release. Do you think that is sufficient?

@EwoutH
Copy link
Member

EwoutH commented Oct 17, 2024

So theoretically this will solve itself with the next pre-release. Do you think that is sufficient?

Yeah for now I do think so. It’s something we need to keep in mind while developing these notebooks.

@Corvince Corvince merged commit 8c67338 into main Oct 17, 2024
11 of 12 checks passed
@jackiekazil jackiekazil added the enhancement Release notes label label Oct 18, 2024
@quaquel
Copy link
Member

quaquel commented Oct 18, 2024

@quaquel Do you have any idea why this might be failing on RTD? Is rtd doing something special or building mesa differently?

The error persists in my work on docs so there is more going on here than just an incorrect version of MESA being installed. I'll try to find a wor arround for now because it gets in the way of testing examples in docs, but we need to figure out what is going on here and properly fix it.

@Corvince
Copy link
Contributor Author

@EwoutH

Actually, the relative imports I introduced here were a bad idea. If you copy the files to a different folder it will not work due to "cannot import relative without a known parent module", i.e. you would additionally need an (empty) init.py file. I don't know what the best solution is here.

@EwoutH
Copy link
Member

EwoutH commented Oct 22, 2024

Yeah, when porting the advanced examples over I now also get constantly:

ImportError: attempted relative import with no known parent package

We need to take another look at this.

@Corvince Corvince deleted the importable-examples branch October 22, 2024 06:51
@Corvince
Copy link
Contributor Author

Corvince commented Oct 22, 2024

You are right. Just to write down the requirements:

  1. Run solara run app.py in the example folder.
  2. Import from mesa.examples import MODELNAME from any folder.
  1. works with an import in app.py in the form of from model import MODELNAME
  2. works in the form of from .model import MODELNAME

Both work with ???

@EwoutH
Copy link
Member

EwoutH commented Oct 22, 2024

The absolute brute force is:

try:
    # Attempt to import when running as part of the package
    from .model import MODELNAME
except ImportError:
    # Fallback to import when running directly
    from model import MODELNAME

The alternative is that we drop requirement 2, and require full-path imports if not from the example folder itself. Like:

from mesa.examples.advanced.epstein_civil_violence.agents import Citizen, Cop
from mesa.examples.advanced.epstein_civil_violence.model import EpsteinCivilViolence

@EwoutH
Copy link
Member

EwoutH commented Oct 22, 2024

I blew about half of my weekly ChatGPT o1-preview budget on this, and we keep coming down to one of three solutions:

  1. Adjusting the Python path so the interpreter can find agents when running the script directly.
    import sys
    import os
    
    sys.path.append(os.path.dirname(os.path.abspath(__file__)))
    
    from .agents import Citizen, Cop
  2. Trying both import styles to handle different execution contexts gracefully.
    try:
        from agents import Citizen, Cop
    except ImportError:
        from .agents import Citizen, Cop
  3. Using an absolute import to explicitly specify the module's location within the package.
    from mesa.examples.advanced.epstein_civil_violence.agents import Citizen, Cop

@quaquel
Copy link
Member

quaquel commented Oct 22, 2024

when you say running the script, do you mean app.py? and do you mean running from the command line?

@Corvince
Copy link
Contributor Author

Corvince commented Oct 22, 2024

Here is another requirement I would like to see fullfilled: Copy the example folder somewhere else (e.g. pycafe, user project) and still be able to use it. Therefore we can't use 3). Since we (currently) only run into this when we run app.py through solara, @maartenbreddels any ideas?

/edit btw the relative imports work when running on py.cafe

@EwoutH
Copy link
Member

EwoutH commented Oct 22, 2024

Also posted on discuss.python.org: https://discuss.python.org/t/making-python-scripts-work-both-as-imports-and-executables-is-there-a-clean-solution/68766/1.

If mesa is installed (with pip) or just cloned also matters a lot.

@maartenbreddels
Copy link

Would solara run mesa.examples.advanced.epstein_civil_violence.app not work? Solara support both scripts and modules.

@Corvince
Copy link
Contributor Author

To move this forward I would suggest changing the imports to absolute imports for now (import mesa.examples.basic.boltzmann_wealth.model). That allows running with solara and being importable. The only thing that's not without additional work is copying the examples into other folders. But that's probably the least important workflow and we can still think about a way to copy/download models as a user (and adjusting imports on the way)

@EwoutH
Copy link
Member

EwoutH commented Oct 24, 2024

Got a few suggestions at discuss.python.org.

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

Successfully merging this pull request may close these issues.

5 participants