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

update history for 2.2.0 release #1917

Closed
wants to merge 0 commits into from
Closed

Conversation

tpike3
Copy link
Contributor

@tpike3 tpike3 commented Dec 19, 2023

Updates history and init for 2.2.0 release.

Copy link

codecov bot commented Dec 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3b326c8) 77.35% compared to head (5cf6a44) 79.62%.
Report is 34 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1917      +/-   ##
==========================================
+ Coverage   77.35%   79.62%   +2.26%     
==========================================
  Files          15       15              
  Lines        1007     1124     +117     
  Branches      220      244      +24     
==========================================
+ Hits          779      895     +116     
- Misses        197      198       +1     
  Partials       31       31              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@EwoutH
Copy link
Contributor

EwoutH commented Dec 19, 2023

Thanks for writing it up!

I know it's a bit ironic since I pushed for a quick release earlier, but it might be worth waiting to see where the discussion in #1912 goes. The most important thing is that we might want to let model.agents return an AgentSet. If we release 2.2.0 with the current implementation, we have to break, deprecate and go straight to 3.0. If we don't release the current behavior, don't have to do all that.

This is an good example of where an experimental release policy would help (#1909). Maybe we just want every feature to be experimental for (for example) two months and at least one minor release, whichever is longer (this is how Rust does is AFAIK).

So if we do want to release 2.2.0 in the short term I would suggest making both model.agents and the DiscreteEventScheduler experimental features.

@jackiekazil
Copy link
Member

@EwoutH agree on holding.
@tpike3 can u close and we will reopen later? I think the history work is good & don't want that to be lost.

@EwoutH
Copy link
Contributor

EwoutH commented Dec 20, 2023

(don’t need to close, maybe mark it as draft)

@EwoutH
Copy link
Contributor

EwoutH commented Dec 22, 2023

What I think needs to be done before release:

Edit: All features are in, ready to start the release process!

Move to next release:

@Corvince
Copy link
Contributor

That's quite a substantial list, but if we check all those boxes it is going to be one of the most exciting releases imho

@EwoutH
Copy link
Contributor

EwoutH commented Dec 23, 2023

Tutorial is modified to such extend that porting over the changes from #1717 will be a significant effort, so let's do that another time.

With #1916 and #1924 merged, we're getting close!

@quaquel
Copy link
Contributor

quaquel commented Dec 24, 2023

I just ran some of the mesa-examples to test the reimplementation of the scheduler. Not all of them work with the current master branch. The key problem seems to be that it is now required to call super().__init__() in your model because of the need to set up the _agent data structure.

@EwoutH
Copy link
Contributor

EwoutH commented Dec 24, 2023

Yes, I also noticed this. It's a bit weird you inherit from Model, it has an init, but it isn't always ran.

But yeah, this is breaking behavior, so we should announce it clearly. Because the fix is so easy if we put it at the top at the release notes it should be fine for a minor release I think.

@tpike3
Copy link
Contributor Author

tpike3 commented Dec 25, 2023

@jackiekazil sorry I just saw your note on closing this, however, I updated it so we can 2.2.0 release

@Corvince
Copy link
Contributor

Yes, I also noticed this. It's a bit weird you inherit from Model, it has an init, but it isn't always ran.

But yeah, this is breaking behavior, so we should announce it clearly. Because the fix is so easy if we put it at the top at the release notes it should be fine for a minor release I think.

Oh that's bad. No I strongly oppose to squeeze this into a minor release. If it is already breaking the examples I think it is very clear that it will also break user models. Not everyone reads release notes and while right now it would be easy to spot in the release notes, of someone would move from say 2.1 to 2.5 it will be hard for them to spot the breaking change.

And we are committed to semver, so users have a legitimate expectation that we don't break their models in minor releases.

This shows that we should really ramp up our ci to automatically run the examples as part of our tests.

For the schedulers I wonder if we could build them in a way that they check if _agents exist and if not create them on the fly? Together with a warning that users should instantiate their models properly and this will be required in 3.0 (when we remove this extra bit of code)

@quaquel
Copy link
Contributor

quaquel commented Dec 25, 2023

For the schedulers I wonder if we could build them in a way that they check if _agents exist and if not create them on the fly? Together with a warning that users should instantiate their models properly and this will be required in 3.0 (when we remove this extra bit of code)

This is indeed an easy fix for the scheduler. The better fix is to handle it in the Agent class because the error starts with instantiating agents that then try to register themselves with the model. This fails because model._agents is not defined.

Also, there is currently still a bug in model. get_agents_of_type

Master currently has

    def get_agents_of_type(self, agenttype: type) -> AgentSet:
         """Retrieves an AgentSet containing all agents of the specified type."""
         return AgentSet(self._agents[agenttype].values(), self)

which should be self._agents[agenttype].keys(),. I have it fixed in the scheduler branch, but I can pull it into its own separate PR if desired.

@jackiekazil
Copy link
Member

Oh that's bad. No I strongly oppose to squeeze this into a minor release. If it is already breaking the examples I think it is very clear that it will also break user models. Not everyone reads release notes and while right now it would be easy to spot in the release notes, of someone would move from say 2.1 to 2.5 it will be hard for them to spot the breaking change.

^^ I like the change, but I agree with this. Maybe it is time to plan 3.0 formally and push the rest of the stuff through on a minor release.

@EwoutH
Copy link
Contributor

EwoutH commented Dec 26, 2023

Agreed, we should be following SemVer. With full hindsight, we probably jumped to 1.0 too quick. But we have to deal with that now.

@quaquel could you open a PR for the fix? Curious what it looks likes.

I would think if we can add a deprecation warning for subclassing Model without running super().__init__() in 2.2, the next release could be 3.0 in which we require it.

@quaquel
Copy link
Contributor

quaquel commented Dec 26, 2023

@quaquel could you open a PR for the fix? Curious what it looks likes.

Done, see #1928.

@EwoutH
Copy link
Contributor

EwoutH commented Dec 27, 2023

Now that Corvince shouted "PositionSet!" the genie is out of the bottle and I can't supress this very intrusive thought any longer. So while - ironically - the PropertyLayer lead to agent movement, leading to agent selection, leading to agent sets, I'm proposing we delay the PropertyLayer (#1898) to the next release. I would like to research both PositionSets as how to apply it on a ContiniousSpace, and both require a major effort.

That means - from my perspective - all features for 2.2.0 are in and we can start the release process. It will be an amazing release!

(I'm willing to help with the release, if desired. I've developed a really nice workflow we use at the EMAworkbench, which I could demonstrate)

@EwoutH
Copy link
Contributor

EwoutH commented Dec 28, 2023

@jackiekazil and I quickly discussed the 2.2.0 release. #1925 was merged, which makes generating the release notes a lot easier.

This is a checklist for the 2.2.0 release.

  1. Make sure all PRs have a clear title and are labeled with at least one label. These will be used when drafting the changelog using the .github/release.yml configuration.
  2. Go to Releases in the GitHub UI and press the Draft a new release button
  3. Set the upcoming tag in the Choose a tag and Release title (i.e. 2.2.0) fields
  4. Use the Generate release notes button to automatically create release notes. Review them carefully.
  5. Write a Highlights section with the most important features or changes in this release (and any other relevant sections).
  6. Copy the release notes and save them with the grey Save draft button.
  7. Open a new PR in which the version number in mesa/init.py is updated (to i.e. "2.2.0" and the copied release notes are added to the HISTORY.md.
  8. Once this PR is merged, go back to the Releases section and Publish the draft release.
  9. The release.yml CI workflow should now automatically create and upload the package to PyPI. Check if this happened on PyPI.org.
  10. Finally, open a new PR in which the version number in mesa/init.py is updated towards the next release (i.e. "2.3.0-dev").

I will write the section for step 5, @jackiekazil can you take care of the other steps? (I can't since I'm not a maintainer)

@EwoutH
Copy link
Contributor

EwoutH commented Jan 1, 2024

Happy new year everyone!

Assuming we're including #1898, I wrote a draft for the 2.2.0 release Highlights. They can be inserted in the release notes in step 5 of the steps above.

@tpike3
Copy link
Contributor Author

tpike3 commented Jan 2, 2024

Happy new year everyone!

Assuming we're including #1898, I wrote a draft for the 2.2.0 release Highlights. They can be inserted in the release notes in step 5 of the steps above.

Apologies for the delay, I was on vacation and did not have my computer. However, my understanding is to release 2.2.0 we need to merge:

Then we can do the release using the new release set up by @EwoutH

Also we should have another update for the visualizations via #1902 in the next 2 weeks.

As our next dev session is 13 Jan, what if we go over the release then and then release at that time?

@EwoutH , @jackiekazil , @quaquel, @Corvince , @rht thoughts?

@EwoutH
Copy link
Contributor

EwoutH commented Jan 2, 2024

No apologies neede, hope you had a nice vacation!

Let's see how fast we can merge #1898 and #1926, and than IMHO release 2.2.0 as soon as we're ready.

@tpike3
Copy link
Contributor Author

tpike3 commented Jan 3, 2024

No apologies neede, hope you had a nice vacation!

Let's see how fast we can merge #1898 and #1926, and than IMHO release 2.2.0 as soon as we're ready.

I am OK with this but I will hold on sending the note to the user group until we get the visualizations integrated.

I would say one critical thing to do prior to the release is can you update the tutorial so it uses the new space and AgentSet features. Does that make sense to you?

On that note (and we can talk at the Dev meeting) is now we need to update all the examples to reflect these very awesome updates. So @jackiekazil for the Dev meeting....

  • Create Mesa examples pypi repo
  • update Mesa examples to include Mesa 2.2.0 features

@EwoutH
Copy link
Contributor

EwoutH commented Jan 3, 2024

I would suggest only updating the main tutorials with stable features. In think for experimental features the docsting + PR description should be enough. It would also undermine the quick and lean development flow for experimental features if tutorials need to be updated immediately.

@tpike3
Copy link
Contributor Author

tpike3 commented Jan 5, 2024

I would suggest only updating the main tutorials with stable features. In think for experimental features the docsting + PR description should be enough. It would also undermine the quick and lean development flow for experimental features if tutorials need to be updated immediately.

Hmmmm, this seems to be a catch-22 we need to add new features and we need users to try them out, but they are experimental. What if we add a new tab to the docs called "New features" or similar and use the introduction tutorial as the pivot so once/if those features become permanent we can just make that the new introduction. I am open to ideas, let me know your thoughts.

@quaquel
Copy link
Contributor

quaquel commented Jan 5, 2024

The current tutorial shows the basics of working with MESA. It thus should reflect the current best practices of using the library, even if this best practice reflects something currently still experimental. There is already precedence for this with the visualization tutorial.

For example, currently, the tutorial does not call super in model.__init__. This is bad practice, and thus, at a minimum, the tutorial should be updated to fix this.

Are there other places to reflect new best practices in the tutorial? Yes, in calculate_gini, it currently has agent_wealths = [agent.wealth for agent in model.schedule.agents]. This should be updated to agent_wealths = [agent.wealth for agent in model.agents] or even better, agent_wealths = model.agents.get('wealth'). This reflects the new AgentSet behavior and is the new preferred way of doing this.

I can't answer for the space side of things and leave that to @EwoutH. But if there are places in the current tutorial for which there is a new, even experimental, but better way of doing it, this should be reflected in the tutorial.

@EwoutH
Copy link
Contributor

EwoutH commented Jan 5, 2024

I would at least not require the tutorials to be needed to be updated, to keep a quick pace. But maybe it can be done optionally.

I’m a bit in doubt what it then even means for a feature to be experimental.

@quaquel
Copy link
Contributor

quaquel commented Jan 5, 2024

  1. tutorials should always work and ideally not raise deprecation warnings.
  2. Whenever a feature it raises a warning to that effect?

@wang-boyu
Copy link
Member

I would imagine the readthedocs site as the main reference for users to know how to use Mesa. It's hard to expect users to go through source code and all examples to figure out new cool experimental features.

As an example, Seaborn introduced extensively their new experimental seaborn.objects interface in their documentation: https://seaborn.pydata.org/tutorial/objects_interface.html

IMHO it's better to have a dedicated page in readthedocs for all experimental features, not just to gain users' attention and collect feedbacks, but also for ourselves to keep tract of them in case we forget their existence. (By the way, would it make sense to group all experimental code into the experimental folder? Or would this make development unnecessarily more complicated?)

I would, however, try not to mix them with main tutorial as much as possible. Being experimental by definition means they are unstable (I might be wrong about this) and could potentially be removed completely. If they become mature enough then we can move them out of experimental and update the main tutorials accordingly. As @tpike3 mentioned, having a page for experimental features may ease the transition process.

@wang-boyu
Copy link
Member

Oh I forgot to mention that I don't think these documentations for experimental features are release blockers, even if we choose to write them. And as always, I'm open to more discussions : )

@Corvince
Copy link
Contributor

Corvince commented Jan 5, 2024

I’m a bit in doubt what it then even means for a feature to be experimental.

I think this is the key point and I also think it is different for all things we have experimental here.

For example the solara Frontend should definitely be the future, but it lacks experience and some features, so in that sense it's experimental.

The property layers add a lot of new functionality and I am sure there are some bugs or corner cases that aren't properly addressed yet, so that's experimental in that sense.

And the new model.agents property and AgentSet in general is just new and also not tested a lot, but I expect it to stay and be incorporated in different building blocks.

@EwoutH
Copy link
Contributor

EwoutH commented Jan 6, 2024

Maybe we need different categories or stages of experimental features. But also don’t make it too complicated.

Anyway, now that #1898 is in, can we do the release?

@rht
Copy link
Contributor

rht commented Jan 7, 2024

#1756 needs to be merged, while we still have the chance.

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.

None yet

7 participants