mumax / 3

GPU-accelerated micromagnetic simulator
Other
447 stars 150 forks source link

Feature/gonumplot #272

Open JeroenMulkers opened 3 years ago

JeroenMulkers commented 3 years ago

This pull request changes how the plot in the web gui is created. Instead of calling gnuplot, the gonum/plot package will be used to create the plot.

The advantages of using gonum/plot over gnuplot are:

godsic commented 3 years ago

Looks neat, but I will give it a closer look.

godsic commented 3 years ago

@JeroenMulkers I've noticed a few things that alarmed me a bit

kkingstoun commented 3 years ago

I can confirm that the gonum's plot works very well on Windows. I have also seen several times "broken pipe" errors.

godsic commented 3 years ago

@JeroenMulkers In addition, I would suggest to not add brackets to labels if units are empty.

JeroenMulkers commented 3 years ago
  1. The update interval of the plot can not be set explicitly. A possible solution is to cache to plot figure and give it a certain expiration date (e.g. 1s). This is what is done in commit #24c775e.

  2. I also changed the data format of the image from svg to png because the svg file can become quiet large (~Megabytes) when the amount of data increases, whereas a png file remains small (kilobytes).

These two changes drastically reduce the number of broken pipe errors. However, there are still disadvantages of using gonum/plot over gnuplot:

I will consider the attempt of changing the plot engine as a fun exercise, but for the above mentioned disadvantages I believe it is better to stick with gnuplot.

godsic commented 3 years ago

@JeroenMulkers I still think it would be a good idea to have go-native visualization, assuming the backend is properly maintained. gonum appears to be a good choice, but go-plotly might be a good alternative assuming we are using offline mode to avoid data breaches to 3rd parties. In any case, @JeroenMulkers would you be so kind to split https://github.com/mumax/3/pull/272/commits/24c775e74decc846f3311bc76c7fa7e1d3147542 into self-contained ones, so that, it would be easier to revive this feature?