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

Changes to AgentSetDF and AgentsDF before time.py -> CopyMixin #16

Merged
merged 20 commits into from
Jul 6, 2024

Conversation

adamamer20
Copy link
Collaborator

@adamamer20 adamamer20 commented Jul 1, 2024

  • Created a CopyMixin for copy and deepcopy methods to let both AgentSet and Scheduler inherit fast copy.
  • Added possibility of using 'do' method with a mask in AgentSetDF, such that the Scheduler can use it to run the method only on a subset of agents (active agents in the scheduler).
  • Added tests for AgentsDF
  • Other minor bug fixes, including type hints and docstrings.

mesa_frames/time.py. Created a CopyMixin for copy and deepcopy methods
to let both AgentSet and Scheduler inherit.
Added possibility of using 'do' method with a mask in AgentSetDF,
such that the Scheduler can use it to run the method only on a
subset of agents.
Minor fixes to type hints and docstrings.
@adamamer20 adamamer20 marked this pull request as draft July 1, 2024 20:31
@adamamer20 adamamer20 mentioned this pull request Jul 2, 2024
@rht
Copy link
Contributor

rht commented Jul 2, 2024

It looks like pre-commit.ci hasn't been enabled: https://pre-commit.ci/.

@adamamer20
Copy link
Collaborator Author

You're right, now it should be enabled.
So now before committing i should run locally pre-commit run --all-files right?

@rht
Copy link
Contributor

rht commented Jul 2, 2024

So now before committing i should run locally pre-commit run --all-files right?

For now, yes, manually. Because it will only trigger starting in the next PR. You could also add a local Git commit hook so that it is automatic.

@adamamer20 adamamer20 marked this pull request as ready for review July 2, 2024 17:59
@adamamer20
Copy link
Collaborator Author

adamamer20 commented Jul 2, 2024

@rht
The ubuntu 3.10 build is failing because typing.Self was introduced in 3.11. I have seen that this can port typing features to previous Python versions. Do you think it's worth it?

@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

@rht rht force-pushed the masked_do_agentset branch from 02ea4d1 to ba7d17b Compare July 3, 2024 01:57
@rht
Copy link
Contributor

rht commented Jul 3, 2024

I tried adding from __future__ import annotations in agents.py. Didn't work. So I have removed my commit so that you are the sole author of this PR (otherwise it would get interpreted as 50% co-authorship without careful inspection).

Adding typing-extensions sounds good to me. Seems like a lightweight module.

@adamamer20 adamamer20 added this to the 1.0.0 Alpha Release milestone Jul 3, 2024
@adamamer20 adamamer20 added the enhancement Improvements to existing features or performance. label Jul 3, 2024
@adamamer20 adamamer20 self-assigned this Jul 3, 2024
@adamamer20 adamamer20 marked this pull request as draft July 3, 2024 15:28
@adamamer20 adamamer20 marked this pull request as ready for review July 4, 2024 18:40
@adamamer20
Copy link
Collaborator Author

@rht ready to merge

@adamamer20 adamamer20 changed the title Changes to AgentSet and AgentsDF before time.py Changes to AgentSetDF and AgentsDF before time.py Jul 5, 2024
@adamamer20 adamamer20 changed the title Changes to AgentSetDF and AgentsDF before time.py Changes to AgentSetDF and AgentsDF before time.py -> CopyMixin Jul 5, 2024
@rht
Copy link
Contributor

rht commented Jul 6, 2024

Added possibility of using 'do' method with a mask in AgentSetDF, such that the Scheduler can use it to run the method only on a subset of agents (active agents in the scheduler).

Sorry for the late comment on this, but the scheduler (random activation, staged activation, etc, except for the discrete event scheduler) will be rendered obsolete, replaced with AgentSet's select-then-do.

But given that this PR has lots of other improvements, e.g. fixing the CI, I'm going to merge it, so that we can move faster. I need a working tests/CI in order to benchmark the runtime type checking libraries.

@rht rht merged commit bf1ccd9 into main Jul 6, 2024
6 checks passed
@rht rht deleted the masked_do_agentset branch July 6, 2024 10:37
@adamamer20
Copy link
Collaborator Author

Sorry for the late comment on this, but the scheduler (random activation, staged activation, etc, except for the discrete event scheduler) will be rendered obsolete, replaced with AgentSet's select-then-do.

But given that this PR has lots of other improvements, e.g. fixing the CI, I'm going to merge it, so that we can move faster. I need a working tests/CI in order to benchmark the runtime type checking libraries.

Ah, no problem! I thought that the scheduler would be kept and the AgentSet's select-then-do was only incentivized. Anyway, all the changes in the PR are still useful because masked do is something that is needed anyway. I have implemented the interface for schedulers (apart from the discrete event scheduler, which I was leaving to its own PR) in the 9-refactoring-mesatime-schedulers branch. I still need to implement the tests, though. At this point, if you plan on deprecating soon, I will leave it as is and start working either on mesa.space or the discrete event scheduler. What do you think?

@rht
Copy link
Contributor

rht commented Jul 6, 2024

I think I'm more looking forward to the mesa.space first, as it will unlock us a more nontrivial example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features or performance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants