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

Plot recipes #27

Open
SebastianM-C opened this issue Nov 26, 2021 · 7 comments
Open

Plot recipes #27

SebastianM-C opened this issue Nov 26, 2021 · 7 comments

Comments

@SebastianM-C
Copy link

Hi! Thanks for writing this package. I was looking over the code and I noticed that for plotting the GLMakie backend is hardcoded. I think that if would be better to define (Makie) plot recipes for PlasmaSolution types and thus depend on Makie / MakieCore and not on a specific backend. This will mean that WGLMakie will work "for free" and in the case of 2D plots, CairoMakie too.

I'd like to help with a PR that implements this if you want.

@killah-t-cell
Copy link
Owner

That's a great idea! The current version is pretty hacky 😄

Feel free to open a PR. I can help if you have any questions as well.

@SebastianM-C
Copy link
Author

Do you have an estimate for how long a simulation should take? I tried running examples from the README and from the tests, but they did not complete (I left them to run for couple of hours).

@killah-t-cell
Copy link
Owner

killah-t-cell commented Dec 1, 2021

They are very heavy. I'm working on making them faster right now – they currently take a few hours. My tip is to test changes with as low a dimension as possible (so 1D1V).

10x speed improvements are my current focus (basically integration is really inefficient right now, and that can be sped up).

@SebastianM-C
Copy link
Author

Do you have a MWE for that? I tried using the example in runtests.jl but I saw that the loss decreased for some time and then started increasing.

With regards to performance, I saw some things that might cause type instabilities, but I'm not sure if those are optimized by MTK when constructing the equations symbolically.

@killah-t-cell
Copy link
Owner

I'll push some pre-trained weights and an MWE asap :)

What do you see that would cause type instabilities?

@SebastianM-C
Copy link
Author

What do you see that would cause type instabilities?

I did not look too close at the code, but

Plasma.jl/src/solve.jl

Lines 68 to 72 in 4147632

qs, ms = [], []
for s in species
push!(qs,s.q)
push!(ms,s.m)
end

might lead to some Anys because qs and ms are initialized as Vector{Any}. I'm not sure if this will create any notable performance problems though (I didn't benchmark or profile the code yet).

I'll try to take a close look at the code and open an issue if I find something else.

@killah-t-cell
Copy link
Owner

Oh nice catch. I don't think this will cause problems, but I think normalisation would matter more here. Setting m to 1 instead of to the electron mass, for example.

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