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

Mountain plot SVGs get too big #121

Open
colleenjg opened this issue Jan 14, 2025 · 3 comments
Open

Mountain plot SVGs get too big #121

colleenjg opened this issue Jan 14, 2025 · 3 comments

Comments

@colleenjg
Copy link
Contributor

I've been trying to save some timeseries plots as SVGs, and encountered a persistent problem.

I'm running a 5 min (10k pt) simulation for about 40 neurons, plotting the timeseries and saving the image, but the SVG is broken half the time (parser error: premature end of data). When I extend the simulation to 10-20 min, it fails consistently (always silently, unfortunately...). The cause is probably that the plotted data is huge and pyplot is (silently!) failing to save to the svg file.

Why is the data so big? This is due to the fill_between part of the mountain plots. It looks like this is a known issue on pyplot's end (see this issue that was active in 2022, but never closed. The solutions alluded to did not work in my hands.)

So this is not an inherent RiaB problem. But since, I would still like to save my svgs, I looked for a workaround on the plotting side. Rasterizing the fill_between part of the plot cuts down the size of the file by about 90% (87 -> 6 MB for my biggest one). So it would be great to be able to rasterize this part specifically (rasterizing the whole plot defeats the purpose of saving as an SVG, whereas rasterizing just one part enables the rest to still be handled as vectors).

The easiest way I can find to do this is to enable users to set the rasterize keyword arg for fill_between(). plot_timeseries() already passes kwargs to utils.mountain_plot(). I could add a keyword arg to utils.mountain_plot() like rasterize_fill=False and pass it on to fill_between(). (Note that currently, just passing rasterize=True as a kwarg would pass on to the plot() function, not the fill_between()). This would lead to no changes for other users, but allow users in my situation (i.e., me :P) to circumvent this problem.

If this feels too niche, another option is to allow keywords for fill_between() to be passed, e.g., shade_kwargs. I can throw my rasterize=True into that, and for others it just gives them extra control over the mountain plot.

Thoughts?

@TomGeorge1234
Copy link
Collaborator

Hey!! I didn't know about this but it's an interesting bug and we should fix it for sure. I would lean towards adding a shade_kwargs param so it's clearer where kwargs are headed. Although we haven't been the most rigorous with this in the past so if you prefer the first way thats fine too...

What does rasterisation mean in this context. For example what happens if I try and break the image into its constituents in Adobe?

@colleenjg
Copy link
Contributor Author

shade_kwargs sounds like a good idea! Will be more versatile for different needs and doesn't center this very specific case as much.

In this context, rasterisation should mean that the specific object plotted (but not the entire subplot) will be rendered as a pixel-based image instead of a vector-based object. So then if you open the saved file in Adobe, although you can still break everything else into its constituents, you can't handle the shading as a vector object. It's not great, but it is a common workaround for dealing with objects that are just too big, which unfortunately the shading can become for large amounts of data due to this bug in pyplot. So we definitely wouldn't want to be rasterizing the shading by default. Just making it possible in cases like mine where the svg isn't even saving properly.

So using shade_kwargs would be perfect as it would allow me to use this workaround as needed without the potential confusion or implied recommendation that adding rasterize_shading=False as a keyword arg would introduce.

Shall I proceed?

@TomGeorge1234
Copy link
Collaborator

Proceed🫡!

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

No branches or pull requests

2 participants