Closed Datseris closed 4 years ago
p.s.: DynamicalBilliards
has plotting as well, but it is hidden behind a Requires
block, which means that PyPlot is neither a dependency nor it needs to be installed to use DynamicalBilliards
. I think we should do the same here.
Also, even though the plotting functionality of DynamicalBilliards
is quite extensive, see https://juliadynamics.github.io/DynamicalBilliards.jl/dev/visualizing/ , it only requires PyPlot
(instead of several plotting packages).
We should be able to apply a similar approach here.
I'm all for making this as light as possible. It is common to have something like Agents.jl
be a big all-encompassing package with something like AgentsCore.jl
that just has the bare minimum for simulations and AgentsPlots.jl
for plotting and Agents.jl
includes AgentsCore.jl
and AgentsPlots.jl
, etc.
Yes this is the second alternative besides Requires
.
I agree that Agents.jl is a heavy package. The following dependencies are not needed for constructing and running simulations:
And all of them are for plotting, including CSV
. It is used to read the color_names.csv
file.
Compose, Cairo and Fontconfig are only there to make saving in PDF format possible. For scientific publications, the already supported PNG is not good enough.
As for using Requires
or AgentsPlots.jl
, I have no experience with either approach and cannot tell the pros and cons. But if we choose to put plotting functions in AgentsPlots.jl
and let Agents.jl
call AgentsPlots.jl
, wouldn't we have the same heaviness when someone calls using Agents
?
To quickly clarify on the AgentsPlots.jl: in this scenario there are three packages: AgentsCore.jl (does the simulations), AgentsPlots.jl (does plotting) an Agents.jl (re-exports the other two).
I dislike this scenario as making 3 packages is really difficult to maintain.
Can you elaborate on the PDF format? Which packages offers the PNG format? Maybe we can find a better alternative in a single package that plots LightGraphs
and offers both formats.
I'm sorry, it is in the documentation of GraphPlot to use Compose for saving to file:
using Compose
draw(PDF("karate.pdf", 16cm, 16cm), gplot(g))
@kavir1698 for VegaLite and DataVoyager, i think simply adding a documentation example which uses these two is enough, we don't have to include them as a dependency.
Since you have experience in this, do you think you could add a paragraph to one of the examples that uses them in an illustrative way? Then the users will be aware that using these two packages is a straightforward process, without having to pay the price of having them as a dependency.
@kavir1698 if Compose
is necessary for .pdf
, where are Cairo, Fontconfig
used?
wouldn't we have the same heaviness when someone calls using Agents?
The idea is that if I don't care at all about plotting or saving figures and only want a lightweight code for simulation purposes, I wouldn't do using Agents
, I'd do using AgentsCore
. This is similar to PlotlyJS.jl. I never do using PlotlyJS
, I only ever do using PlotlyBase
. If I were to use this code, I would similarly only ever do using AgentsCore
and would hope it is as light as possible.
We can just drop VegaLite
altogether, because it is only used in one plotting function that is not that difficult. Users can make their own plots. Same for DataVoyager
. I will add one line to the tutorial mentioning that since data are in a DataFrame
, one can easily plot them with DataVoyager
.
If I remember correctly, I had to add Cairo and Fontconfig to prevent some errors from Compose
. They are not used in my code directly. Maybe we can drop them too.
Here is a candidate that we could consider, because it is detached from the plotting backend: https://github.com/JuliaPlots/GraphRecipes.jl
Cool!
One other possibility is to keep the visualization functions as they are but remove Cairo
, Compose
, and Fontconfig
from dependencies. In the documentation, before using a visualization function, we import Compose
. This is the same approach as the GraphPlot
package.
We have already dropped VegaLite
, DataVoyager
and CSV
. If you agree with this suggestion, we will drop three more dependencies out of eight. We will be only left two dependencies: GraphPlot
and ColorTypes
. We could even remove them, and before using visualization call using Compose, GraphPlot, ColorTypes
It is not hygiene to define functions that use a package without actually using this package in the source code. ALthough it is possible in the Julia language, it does lead to problems. But since only GraphPlot
remains, we put it behind a requires block and its smooth sailing. (i'll take care of this)
Where is ColorTypes
used?
ColorTypes
is used for defining node colors. I will see if I can replace it with custom types.
In addition to GraphPlot
, Compose, Cairo and Fontconfig should go there too. Compose
is needed for saving plots, and Cairo
and Fontconfig
are needed if the saved plot is a PDF.
It is not straightforward to replace ColorTypes
. But it is a small package. If we are keeping Compose
and Cairo
, we might as well keep ColorTypes
.
My vote: Don't keep any and leave plotting to a different package 😅
Maybe the simplest way to go forwards is to define Agents.jl
and AgentsPlots.jl
which re-exports Agents.jl
and defines plotting on top. No need for a third package...
If you are ok with two packages, I might suggest Agents.jl contain everything including plots and AgentsCore.jl containing the minimum stuff required for simulations, but anything is fine includiong just a single package :)
If multiple packages is the way we are going, I would also vote for Agents.jl
to contain the core functions and AgentsPlots.jl
for plotting. The reason is that the plotting functions are only useful for certain simulations: those with a grid space with a maximum of two dimensions. Plus, it is easy to remember to call AgentsPlots
when one wants to make visualizations. But people may forget to call AgentsCore
instead of Agents
if they do not need the visualizations.
I personally don't like multiple packages at all. @kavir1698 before we make a decision on this, can we actually see how much dependencies are actually remaining? From the discussion here seems like everything was dropped, besides ColorTypes.
If there is one top level additional using
call, there is no reason to go with multiple packages. Just
using GraphPlots
would be enough and internally we can use requires...
The remaining dependencies are GraphPlots, ColorTypes, Compose, Cairo, and Fontconfig.
We should also leave room for future extensions, such as real-time plots, interactive plots, and 3D plots.
Is it a good idea to add multiple packages to the "requires" block? If so, I don't see an advantage in creating AgentsPlots.jl
.
I thought Compose, Cairo, and Fontconfig
are only used to save an image as pdf, and thus they are not dependencies. You don't have to depend on the them for our package to work...?
Well, all the functions save the plots, so they wouldn't work without Compose
, Cairo
, and Fontconfig
.
Can you explain to me why you would think Agents.jl should be responsible for saving plots?
(for me this is the point on confusion: what you believe should be offered by a package that does agent based modelling)
Because the gplot
function of GraphPlots
does not display a plot. One has to save it to see it.
Of course, we can create the plot and leave the saving to the user. That is possible.
Hm, I think this is a good point to reconsider: maybe actually having only a single dependency, that allows both plotting, seeing, and saving, is a better approach.
Seems to me that GraphRecipes.jl surpasses everything that has been suggested so far:
The downside of course being:
I am checking GraphRecipes to see whether I can make the same plots there.
If you could hide the GraphRecipes dependency behind a requires
so that the stuff only gets loaded if GraphRecipes is loaded, that seems ok to me (but I am mostly a bystander so feel free to discount my opinion 😅)
The first example of GraphRecipes does not run. I have submitted and issue on their page. https://github.com/JuliaPlots/GraphRecipes.jl/issues/83#issue-519948705
If you could hide the GraphRecipes dependency behind a requires so that the stuff only gets loaded if GraphRecipes is loaded, that seems ok to me
Maybe yes, but maybe no: GrapheRecipes
is lightweight (because it does not actually do plotting), one doesn't have to use Requires for this. Requires is mainly meant for heavy stuff. But maybe we should do it anyway though...
@kavir1698 sorry for this, people suggested GraphRecipes.jl on the graphs slack channel and I took their word for it. I should have checked in more detail...
Plotting is such a touchy Pandora's box. I prefer to handle my own plots so even if I use this package, I certainly won't be using any of its plotting functionality and that extra functionality will likely lead to irrititating bugs and maintenance headaches. Why not just remove it completely?
I wouldn't vote for removing it completely: when I am doing science, I must be able to visualize my results in a basic way. It helps me understand what is going on. For ABM here is what I would like to have:
A function that takes my model and plots it: as a graph. And the color of each node is a user specified function that takes as an input the agents in this node and outputs a color value. I would certainly expect this basic plotting functionality to come from Agents.jl. (and why not? it seems like 20 lines of code to do this).
One just has to be smart about how to handle dependencies and to ensure no actual plotting backend is installed with Agents.jl
We do not make general plots. But displaying cellular automata and agent positions on a grid are non-trivial.
Further, I imagine adding real time plots will be an important step forward for future development of this package. So, plots are valuable features of an agent based modeling framework.
On Fri, Nov 8, 2019, 12:55 PM Eric Forgy notifications@github.com wrote:
Plotting is such a touchy Pandora's box. I prefer to handle my own plots so even if I use this package, I certainly won't be using any of its plotting functionality and that extra functionality will likely lead to irrititating bugs and maintenance headaches. Why not just remove it completely?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaDynamics/Agents.jl/issues/46?email_source=notifications&email_token=AA47OUP5STNVLGRLUHB45ETQSVHTBA5CNFSM4JKG6RD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDQ3DEI#issuecomment-551661969, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA47OUOOFTPTFOH4BD4ZL7LQSVHTBANCNFSM4JKG6RDQ .
@kavir1698 sorry for this, people suggested GraphRecipes.jl on the graphs slack channel and I took their word for it. I should have checked in more detail...
No worries. GraphPlot
works pretty well. The only problem is that it requires Compose
for saving plots. Compose
on the other hand requires Cairo
and Fontconfig
for saving PDFs.
I agree that plotting is important. I can't write code without simultaneously visualizing what I'm doing, but injecting your choice of plotting package into a simulation tool when I don't need or want that plotting package (because I have my own), I just feel like it is unnecessary and asking for headaches. I would be super irritated if I have trouble installing Agents.jl because some plotting package was broken.
I think creating AgentsPlots.jl
will be a good compromise. It reduces extra load times from Agents.jl
and maintaining it at the current level is not much work. It has only a few functions. For the future, when more complicated visualization is added, having a separate plotting package may be inevitable.
Alright, let's go with this then.
Closing for #48
Here is the thing: Agents.jl is a very, very heavy package.
This is not only because it has a vast list of dependencies, but also because these dependencies are very heavy themselves. Some of these dependencies also pose serious problems, e.g. https://github.com/JuliaDynamics/Agents.jl/issues/17 (which I bet will certainly lead to failures at some point)
What really makes me open this issue though is this:
Why would this be bundled in Agents.jl as a dependency? To the best of my understanding, the only thing we do is pass the dataframe to data voyager. Shouldn't this one liner be left to the desire of the user as an extra step?
If we really think about it, the fundamental, science-oriented purpose of Agents.jl should only need
LightGraphs, DataFrames
as dependencies.Here is what I want to propose and discuss in this post: we can very slightly limit the offered extra functionality, but by doing so we gain:
Fontconfig
gets abandoned?" (notice by the way that neither of the authors have replied to us in #17 )What if we limit the extra dependencies so that the only extra dependency is
GraphPlot
, to plot agents on their graph? How much do we really lose by doing that?(I also don't understand why multiple plotting packages
Cairo, GraphPlots
are dependencies of Agents.jl)