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

JupyterViz: Simplify SliderInt and SliderFloat specification using slice #1945

Closed
wants to merge 1 commit into from

Conversation

rht
Copy link
Contributor

@rht rht commented Jan 8, 2024

With this PR, instead of the lengthy model_params

model_params = {
    "density": {
        "type": "SliderFloat",
        "value": 0.8,
        "label": "Agent density",
        "min": 0.1,
        "max": 1.0,
        "step": 0.1,
    },
    "minority_pc": {
        "type": "SliderFloat",
        "value": 0.2,
        "label": "Fraction minority",
        "min": 0.0,
        "max": 1.0,
        "step": 0.05,
    },
    "homophily": {
        "type": "SliderInt",
        "value": 3,
        "label": "Homophily",
        "min": 0,
        "max": 8,
        "step": 1,
    },
    "width": 20,
    "height": 20,
}

This becomes

model_params = {
    "density": {
        "type": "SliderFloat",
        "value": 0.8,
        "label": "Agent density",
        "values": slice(0.1, 1.0, 0.1),
    },
    "minority_pc": {
        "type": "SliderFloat",
        "value": 0.2,
        "label": "Fraction minority",
        "values": slice(0.0, 1.0, 0.05),
    },
    "homophily": {
        "type": "SliderInt",
        "value": 3,
        "label": "Homophily",
        "values": slice(0, 8, 1),
    },
    "width": 20,
    "height": 20,
}

Copy link

codecov bot commented Jan 8, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (0bd4429) 79.87% compared to head (d4d94dc) 79.73%.
Report is 15 commits behind head on main.

Files Patch % Lines
mesa/experimental/jupyter_viz.py 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1945      +/-   ##
==========================================
- Coverage   79.87%   79.73%   -0.14%     
==========================================
  Files          15       15              
  Lines        1267     1278      +11     
  Branches      277      279       +2     
==========================================
+ Hits         1012     1019       +7     
- Misses        216      220       +4     
  Partials       39       39              

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

@EwoutH
Copy link
Member

EwoutH commented Jan 8, 2024

It’s shorter, but also less explicit. So I’m a bit mixed about this.

@rht
Copy link
Contributor Author

rht commented Jan 8, 2024

It's reminiscent of range(1, 24, 2), and so users already have a shared context of what the 3 numbers refer to.

@quaquel
Copy link
Member

quaquel commented Jan 9, 2024

I like the brevity of this. Also, because it is similar to how, e.g., range and slicing work; it is easy to explain. I am a bit less sure about the choice to reuse slice for this. slice is normally used to produce a sequence of indices. Indices, for me, are integers, but that is not the case here. So why did you decide to reuse slice for this?

@rht
Copy link
Contributor Author

rht commented Jan 10, 2024

I agree that using slice does not make sense when the arguments are float.

In [8]: slice(0.1, 0.9, 0.2).indices(1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[8], line 1
----> 1 slice(0.1, 0.9, 0.2).indices(1)

TypeError: slice indices must be integers or None or have an __index__ method

range is more suitable, except that it doesn't support float arguments.

It seems that I will have to define

class arange:
    def __init__(self, start, stop, step):
        self.start = start
        self.stop = stop
        self.step = step
        self.current = start

    def __iter__(self):
        return self

    def __next__(self):
        if self.current >= self.stop:
            raise StopIteration
        else:
            result = self.current
            self.current += self.step
            return result

@quaquel
Copy link
Member

quaquel commented Jan 10, 2024

I indeed see two options:

  • add a small helper class as described by @rht, although I would suggest trying to find a different name because arrange clashes with numpy.arrange.
  • use a tuple or namedtuple with start, end, step, and handle the rest internally within jupyterviz.

@rht
Copy link
Contributor Author

rht commented Jan 11, 2024

use a tuple or namedtuple with start, end, step, and handle the rest internally within jupyterviz.

Regardless of how it should be implemented under the hood, np.arange has the closest definition to how the tuple (start, stop, step) should mean. I think it's fine to reuse the name, as long as the namespace is mesa.experimental.arange or mesa.arange.

@tpike3 tpike3 added docs Release notes label enhancement Release notes label and removed docs Release notes label labels Jan 13, 2024
@rht
Copy link
Contributor Author

rht commented Jan 17, 2024

https://emaworkbench.readthedocs.io/en/latest/examples/example_mesa.html has a more concise description. Instead of

{
    "type": "SliderFloat",
    "value": 0.8,
    "label": "Agent density",
    "min": 0.1,
    "max": 1.0,
    "step": 0.1,
}

we have

from mesa.experimental import `Slider`
Slider("Agent density", 0.8, 0.1, 1.0, 0.1)
# optionally
# min_value instead of min in order to avoid collision with reserved keywords
Slider(name="Agent density", value=0.8, min_value=0.1, max_value=1.0, step=0.1)
  1. Slider encompasses both int and float, and to translate it to Solara, there would be any(insinstance(e, float) for e in (value, min_value, max_value, step))
  2. This ties back to the original Tornado viz interface

@rht
Copy link
Contributor Author

rht commented Jan 17, 2024

We could have named it NumberParameter, so that it can be reused for sensitivity analysis, not just visualization.

@rht
Copy link
Contributor Author

rht commented Jan 18, 2024

Superseded by #1972.

@rht rht closed this Jan 18, 2024
@rht rht deleted the jupyterviz_slice branch January 18, 2024 07:46
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.

4 participants