Optionally deep clone run outputs #1142

dmadisetti · 2024-04-16T18:17:27Z

Describe the bug

I noticed when I did the qmd export for quarto that plots don't properly work:

https://dmadisetti.github.io/quarto-marimo/plots.html
https://dmadisetti.github.io/quarto-marimo/plots-pdf.pdf
It looks like the plots all refer to the global plt.gca object rather than its snapshot at run.

This doesn't affect the reactive notebook since the plot object is rendered to HTML, essentially creating a snapshot, and then the next cell is executed. However, in script mode the output is just a pointer to the current global state

Suggestion: I think deep copies can be expensive, so maybe an optional deep copy?

Environment

3.12-dev (I haven't pulled in a few days but I don't think you've made any changed that would affect this)

Code to reproduce

Relevant export test:

 
import matplotlib.pyplot as plt
 
if __name__ == "__main__":
    objs, _ = app.run()
    for i, o in enumerate(objs):
        # Unpack the Line2d cases
        if isinstance(o, list):
          o, = o
        # Quick hack for testing
        if "matplotlib" in str(type(o)):
          o.get_figure().savefig(f"{i}.png")

for completion the [plots.py](http://plots.py) tutorial

# Copyright 2024 Marimo. All rights reserved.
import marimo

__generated_with = "0.1.69"
app = marimo.App()


@app.cell(hide_code=True)
def __(mo):
    mo.md("# Plotting")
    return


@app.cell(hide_code=True)
def __(check_dependencies):
    check_dependencies()
    return


@app.cell(hide_code=True)
def __(mo):
    mo.md(
        """
        marimo supports several popular plotting libraries, including matplotlib,
        plotly, seaborn, and altair. 

        This tutorial gives examples using matplotlib; other libraries are
        used similarly.
        """
    )
    return


@app.cell(hide_code=True)
def __(mo):
    mo.md("## Matplotlib")
    return


@app.cell(hide_code=True)
def __(mo):
    mo.md(
        """
        To show a plot, include it in the last expression of a cell (just
        like any other output).

        ```python3
        # create the plot in the last line of the cell
        import matplotlib.pyplot as plt
        plt.plot([1, 2])
        ```
        """
    )
    return
 
 
@app.cell
def __(plt):
    plt.plot([1, 2])
    return
 
 
@app.cell(hide_code=True)
def __(mo):
    mo.md(
        """
        ```python3
        # create a plot
        plt.plot([1, 2])
        # ... do some work ...
        # make plt.gca() the last line of the cell
        plt.gca()
        ```
        """
    )
    return


@app.cell
def __(plt):
    plt.plot([1, 2])
    # ... do some work ...
    # make plt.gca() the last line of the cell
    plt.gca()
    return


@app.cell(hide_code=True)
def __(mo, plt_show_explainer):
    mo.accordion(plt_show_explainer)
    return


@app.cell(hide_code=True)
def __(mo):
    mo.md(
        """
        **A new figure every cell.** Every cell starts with an empty figure for 
        the imperative `pyplot` API.
        """
    )
    return


@app.cell
def __(np):
    x = np.linspace(start=-4, stop=4, num=100, dtype=float)
    return x,


@app.cell
def __(plt, x):
    plt.plot(x, x)
    plt.plot(x, x**2)
    plt.gca()
    return


@app.cell
def __(plt, x):
    plt.plot(x, x**3)
    return


@app.cell(hide_code=True)
def __(mo):
    mo.md(
        """
        To build a figure over multiple cells, use the object-oriented API and
        create your own axis:
        """
    )
    return


@app.cell
def __(plt, x):
    _, axis = plt.subplots()
    axis.plot(x, x)
    axis.plot(x, x**2)
    axis
    return axis,


@app.cell
def __(axis, x):
    axis.plot(x, x**3)
    axis
    return


@app.cell(hide_code=True)
def __(mo):
    mo.md(
        """
        ### Draw plots interactively

        Draw plots interactively by parametrizing them with UI elements.
        """
    )
    return


@app.cell
def __(mo):
    exponent = mo.ui.slider(1, 5, value=1, step=1, label='exponent')

    mo.md(
        f"""
        **Visualizing powers.**

        {exponent}
        """
    )
    return exponent,


@app.cell
def __(exponent, mo, plt, x):
    import functools


    @functools.cache
    def _plot(exponent):
        plt.plot(x, x**exponent)
        return plt.gca()


    _tex = (
        f"$$f(x) = x^{exponent.value}$$" if exponent.value > 1 else "$$f(x) = x$$"
    )

    mo.md(
        f"""

        {_tex}

        {mo.as_html(_plot(exponent.value))}
        """
    )
    return functools,


@app.cell(hide_code=True)
def __(mo):
    mo.md("## Other libraries")
    return


@app.cell(hide_code=True)
def __(mo):
    mo.md(
        """
        marimo also supports these other plotting libraries:

        - Plotly
        - Seaborn
        - Altair

        Just output their figure objects as the last expression of a cell,
        or embed them in markdown with `mo.as_html`.

        If you would like another library to be integrated into marimo, please
        get in touch.
        """
    )
    return


@app.cell(hide_code=True)
def __(missing_packages, mo):
    module_not_found_explainer = mo.md(
        """
        ## Oops!

        It looks like you're missing a package that this tutorial 
        requires.

        Close marimo, install **`numpy`** and **`matplotlib`**, then 
        open this tutorial once more.

        If you use `pip`, run

        ```
        pip install numpy matplotlib
        ```

        at your command line.
        """
    ).callout(kind='warn')

    def check_dependencies():
        if missing_packages:
            return module_not_found_explainer
    return check_dependencies, module_not_found_explainer


@app.cell(hide_code=True)
def __():
    plt_show_explainer = {
        "Using `plt.show()`": """
        You can use `plt.show()` or `figure.show()` to display
        plots in the console area of a cell. Keep in mind that console
        outputs are not shown in the app view.
        """
    }
    return plt_show_explainer,


@app.cell
def __():
    try:
        import matplotlib
        import matplotlib.pyplot as plt
        import numpy as np
        missing_packages = False
    except ModuleNotFoundError:
        missing_packages = True

    if not missing_packages:
        matplotlib.rcParams['figure.figsize'] = (6, 2.4)
    return matplotlib, missing_packages, np, plt


@app.cell
def __():
    import marimo as mo
    return mo,

import matplotlib.pyplot as plt

if __name__ == "__main__":
    objs, _ = app.run()
    for i, o in enumerate(objs):
        # Unpack the Line2d cases
        if isinstance(o, list):
          o, = o
        # Quick hack for testing
        if "matplotlib" in str(type(o)):
          o.get_figure().savefig(f"{i}.png")

produces: 11.png 12.png 14.png 15.png 5.png 7.png

where 5 and 7 are the same, and the rest are the same
This isn't the same behavior as in the export, but similar, and still incorrect

The text was updated successfully, but these errors were encountered:

mscolnick · 2024-04-24T02:32:19Z

We do some fancy-ish things in marimo/_output/mpl.py

which is called twice:

marimo/marimo/_output/formatters/matplotlib_formatters.py

Line 25 in b4256c9

matplotlib.use("module://marimo._output.mpl")
marimo/marimo/_runtime/runtime.py

Line 964 in b4256c9

exec("__marimo__._output.mpl.close_figures()", self.globals)

I wonder if this is only run in edit mode (and not as a script) that you are seeing this issue. and possible we need to clear the plots in run mode. @akshayka any thoughts?

dmadisetti · 2024-04-24T15:12:24Z

This does not happen with the way islands generates cells, but I just checked behavior with the script and I now have this error?

Traceback (most recent call last):
  File "/home/dylan/test.py", line 293, in <module>
    objs, _ = app.run()
              ^^^^^^^^^
  File "/home/dylan/src/marimo/marimo/_ast/app.py", line 335, in run
    return self._run_sync()
           ^^^^^^^^^^^^^^^^
  File "/home/dylan/src/marimo/marimo/_ast/app.py", line 267, in _run_sync
    outputs[cid] = execute_cell(cell._cell, glbls)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dylan/src/marimo/marimo/_ast/cell.py", line 444, in execute_cell
    exec(cell.body, glbls)
  File "/tmp/marimo_3443916/__marimo__cell_TqIu_.py", line 11, in <module>
  File "/home/dylan/src/marimo/marimo/_plugins/ui/_core/ui_element.py", line 267, in value
    == ctx.ui_element_registry.get_cell(self._id)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dylan/src/marimo/marimo/_plugins/ui/_core/registry.py", line 200, in get_cell
    raise NotImplementedError
NotImplementedError

mscolnick · 2024-04-24T17:12:38Z

@dmadisetti - this is likely from #1150. do you have a small snippet of the code to reproduce?

dmadisetti · 2024-04-24T17:21:12Z

Not exactly the same error:

Traceback (most recent call last):                                                                                                                                                                                                
  File "/home/dylan/1086.py", line 21, in <module>                                                                                                                                                                                
    app.run()                                                                                                                                                                                                                     
  File "/home/dylan/src/marimo/marimo/_ast/app.py", line 335, in run                                                                                                                                                              
    return self._run_sync()                                                                                                                                                                                                       
           ^^^^^^^^^^^^^^^^                                                                                                                                                                                                       
  File "/home/dylan/src/marimo/marimo/_ast/app.py", line 267, in _run_sync                                                                                                                                                        
    outputs[cid] = execute_cell(cell._cell, glbls)                                                                                                                                                                                
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                                                                                
  File "/home/dylan/src/marimo/marimo/_ast/cell.py", line 445, in execute_cell                                                                                                                                                    
    return eval(cell.last_expr, glbls)                                                                                                                                                                                            
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                                                                                            
  File "/tmp/marimo_3531073/__marimo__cell_Hbol__output.py", line 7, in <module>                                                                                                                                                  
  File "/home/dylan/downloads/manim/lib/python3.11/site-packages/matplotlib/pyplot.py", line 527, in show                                                                                                                         
    return _get_backend_mod().show(*args, **kwargs)                                                                                                                                                                               
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                                                                               
  File "/home/dylan/src/marimo/marimo/_output/mpl.py", line 64, in show                                                                                                                                                           
    _internal_show(manager.canvas)                                                                                                                                                                                                
  File "/home/dylan/src/marimo/marimo/_output/mpl.py", line 47, in _internal_show                                                                                                                                                 
    CellOp.broadcast_console_output(                                                                                                                                                                                              
  File "/home/dylan/src/marimo/marimo/_messaging/ops.py", line 179, in broadcast_console_output                                                                                                                                   
    assert cell_id is not None                                                                                                                                                                                                    
           ^^^^^^^^^^^^^^^^^^^                                                                                                                                                                                                    
AssertionError

might be worth adding this as a unit test: https://github.com/marimo-team/marimo/blob/main/marimo/_smoke_tests/bugs/1086.py

I'll see if I can boil it down to something smaller, but the first stack track is just plots.py from the tutorial with the full source and initial output in the original summary

mscolnick · 2024-04-24T17:29:59Z

1086.py should be good enough to work from - and we can add a unit test for this, thanks

mscolnick · 2024-04-24T21:13:48Z

@akshayka just squashed 3 related bugs. can you give it another shot with your own code? (we have it running again our tutorials now)

dmadisetti · 2024-04-25T01:09:58Z

Yes! This fixes the crash- this also fixes most of the redundant plots in tutorial. However, the shared axis object still has a global response:

@app.cell  # cell 14                                                                                                                                                                                                                           
def __(plt, x):                                                                                                                                                                                                                     
    _, axis = plt.subplots()                                                                                                                                                                                                        
    axis.plot(x, x)                                                                                                                                                                                                                 
    axis.plot(x, x**2)                                                                                                                                                                                                              
    axis                                                                                                                                                                                                                            
    return axis,

@app.cell # cell 15
def __(axis, x):                                                                                                                                                                                                                    
    axis.plot(x, x**3)                                                                                                                                                                                                              
    axis                                                                                                                                                                                                                            
    return

dmadisetti · 2024-04-25T01:15:08Z

This is actually a bug beyond just scripts now:

run order will change the output of this. Correct behavior should be

plot 1: x, x^2 
plot 2: x, x^2, x^3
+ plot 3: x, x^2, x^4
- plot 3: x, x^2, x^3, x^4

dmadisetti · 2024-04-25T01:23:12Z

OR, have the script response as being correct

- plot 1: x, x^2 
- plot 2: x, x^2, x^3
+ plot 1: x, x^2, x^3, x^4
+ plot 2: x, x^2, x^3, x^4
plot 3: x, x^2, x^3, x^4

code:

import marimo                                                                                                                                                                                                                       
                                                                                                                                                                                                                                    
app = marimo.App()                                                                                                                                                                                                                  
                                                                                                                                                                                                                                    
@app.cell                                                                                                                                                                                                                           
def __(np):                                                                                                                                                                                                                         
    x = np.linspace(start=-4, stop=4, num=100, dtype=float)                                                                                                                                                                         
    return x,    

def __(plt, x):
    _, axis = plt.subplots()
    axis.plot(x, x)
    axis.plot(x, x**2)
    axis
    return axis,


@app.cell
def __(axis, x):
    axis.plot(x, x**3)
    axis
    return


@app.cell
def __(axis, x):
    axis.plot(x, x**4)
    axis
    return


@app.cell
def __():
    import marimo as mo
    try:
        import matplotlib
        import matplotlib.pyplot as plt
        import numpy as np
        missing_packages = False
    except ModuleNotFoundError:
        missing_packages = True

    if not missing_packages:
        matplotlib.rcParams['figure.figsize'] = (6, 2.4)
    return mo, matplotlib, missing_packages, np, plt

import matplotlib.pyplot as plt

if __name__ == "__main__":
    objs, v = app.run()
    print(v)
    for i, o in enumerate(objs):
        # Unpack the Line2d cases
        if isinstance(o, list):
          o, = o
        # Quick hack for testing
        if "matplotlib" in str(type(o)):
          o.get_figure().savefig(f"{i}.png")

akshayka · 2024-04-25T04:20:27Z

run order will change the output of this. Correct behavior should be ...
plot 1: x, x^2
plot 2: x, x^2, x^3

plot 3: x, x^2, x^4

plot 3: x, x^2, x^3, x^4

Don't they all share the same axis object though? So it makes sense that plot 3 has x^3.

OR, have the script response as being correct

Okay, I now see how providing a fresh figure isn't sufficient. Thanks for the example. This I suppose isn't specific to plots but really to any data structure that is mutated over and output in multiple cells.

For the particular use case of integrating with Quarto or exporting to HTML — I think islands / marimo export html will now do the right thing, right?
We could still consider adding an optional deep clone to app.run() — but I wonder if there's a better solution. Perhaps we could register a _mime_ method on each output that returned the snapshotted HTML?

dmadisetti · 2024-04-25T13:26:24Z

Correct is relative to design, I think both cases are valid- but ideally you'd want consistency

Yes, export and islands run first and then access output, so Case 2 is what happens—but the result is still cell order dependent particular to matplotlib z-order will change.
I wonder what the performance hit would actually be. Maybe it's fine—premature optimization and all that. Alternatively, what about creating a check to see if a known data structure is in defs and just clones those specific object types?

global hidden state is really hard to get around- the data flow tutorial makes it pretty clear that objects are hard to handle, so this could just be covered under that disclaimer

Just an aside, dataflow expectations for parallelism are definitely Case 1

akshayka · 2024-04-25T16:43:13Z

Correct is relative to design, I think both cases are valid- but ideally you'd want consistency

Yeah, consistency is key.

Export snapshots while running, so I think you get the same semantics as when running as an notebook (at least that's what happened when I ran it just now). But like you say the more important thing is to define the semantics and ensure consistency across different ways of running.

Just an aside, dataflow expectations for parallelism are definitely Case 1

To double check -- what is Case 1? :)

dmadisetti · 2024-04-25T17:35:02Z

Oh woops. To be explicit:

Case 1

graph TD;
    cell1("Cell 1\nShows x, x^2")-->cell2("Cell 2\nShows x, x^2, x^3");
    cell1-->cell3("Cell 3\nShows x, x^2, x^4");

Which can be made parallel because the tasks are independent

Case 2

graph TD;
    cell1("Cell 1\nShows x, x^2, x^3, x^4")-->cell2("Cell 2\nShows x, x^2, x^3, x^4");
    cell3("Cell 3\nShows x, x^2, x^3, x^4");
    cell2 --"order takes precedence in mutating axis"-->cell3;

Case 2 has a hidden dependency since the global object is built up.

akshayka · 2024-04-25T20:58:45Z

Okay great, yes — that makes total sense. On the same page.

dmadisetti · 2024-06-06T15:26:01Z

@akshayka I might take a stab at this over the weekend. I have something working that passes all unit tests with cloning on (except pickling, but I think that's the test env not being patched), but I want to write unit tests for it explicitly/ clean it up. FWIW, I haven't observed a noticeable overhead

My change also solves some of the behavior I observed in #1509 and introduces a mo.zero_copy as a variable-level opt-out wrapper for cloning.

This is all under a user config flag, that I currently have as execution.puritan : bool, but there's probably a better/less religious-sounding flag name. LMK what you think. Maybe execution.determinism : Enum[relaxed, strict]?

I want to get mo.save in, and then I'll make a doc page on "Ensuring Reproducibility".

akshayka · 2024-06-06T16:34:28Z

I have something working that passes all unit tests with cloning on (except pickling, but I think that's the test env not being patched)

Nice! Is this just for app.run(), or for the kernel runtime too?

execution.determinism : Enum[relaxed, strict]

Could be runtime.determinism. I could see this being useful for catching bugs, for example discovering accidental mutations.

I want to get mo.save in, and then I'll make a doc page on "Ensuring Reproducibility".

That would be awesome!

dmadisetti · 2024-06-25T15:52:23Z

Closing with #1580

dmadisetti added the bug label Apr 16, 2024

This was referenced Apr 24, 2024

fix: close matplotlib figures in app.run() #1215

Merged

fix: don't load marimo mpl backend in script context #1216

Merged

fix: give real UIElementRegistry when running as a script #1218

Merged

dmadisetti mentioned this issue Jun 8, 2024

Strict Execution #1580

Merged

3 tasks

dmadisetti closed this as completed Jun 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optionally deep clone run outputs #1142

Optionally deep clone run outputs #1142

dmadisetti commented Apr 16, 2024 •

edited by linear bot

Loading

mscolnick commented Apr 24, 2024 •

edited

Loading

dmadisetti commented Apr 24, 2024

mscolnick commented Apr 24, 2024

dmadisetti commented Apr 24, 2024

mscolnick commented Apr 24, 2024

mscolnick commented Apr 24, 2024

dmadisetti commented Apr 25, 2024

dmadisetti commented Apr 25, 2024

dmadisetti commented Apr 25, 2024

akshayka commented Apr 25, 2024

dmadisetti commented Apr 25, 2024

akshayka commented Apr 25, 2024

dmadisetti commented Apr 25, 2024

akshayka commented Apr 25, 2024

dmadisetti commented Jun 6, 2024

akshayka commented Jun 6, 2024

dmadisetti commented Jun 25, 2024

Optionally deep clone run outputs #1142

Optionally deep clone run outputs #1142

Comments

dmadisetti commented Apr 16, 2024 • edited by linear bot Loading

Describe the bug

Environment

Code to reproduce

mscolnick commented Apr 24, 2024 • edited Loading

dmadisetti commented Apr 24, 2024

mscolnick commented Apr 24, 2024

dmadisetti commented Apr 24, 2024

mscolnick commented Apr 24, 2024

mscolnick commented Apr 24, 2024

dmadisetti commented Apr 25, 2024

dmadisetti commented Apr 25, 2024

dmadisetti commented Apr 25, 2024

akshayka commented Apr 25, 2024

dmadisetti commented Apr 25, 2024

akshayka commented Apr 25, 2024

dmadisetti commented Apr 25, 2024

akshayka commented Apr 25, 2024

dmadisetti commented Jun 6, 2024

akshayka commented Jun 6, 2024

dmadisetti commented Jun 25, 2024

dmadisetti commented Apr 16, 2024 •

edited by linear bot

Loading

mscolnick commented Apr 24, 2024 •

edited

Loading