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

Support options for Variograms #21

Open
PeerHerholz opened this issue Mar 4, 2021 · 4 comments
Open

Support options for Variograms #21

PeerHerholz opened this issue Mar 4, 2021 · 4 comments

Comments

@PeerHerholz
Copy link

Ahoi hoi everyone,

thanks for this amazing toolbox and great documentation.

While checking the fit of my surrogate maps, I noticed that I can't specify any figure options for the variogram. While I think the plots are already great as is, adding support for changing the figure size, font size and maybe colormap could/would be useful. This could potentially be done via kwargs that are utilized during the plot generation.
I would be happy to submit a PR for that, given that this is something you would like to integrate.

Thanks again, please stay safe.

Cheers, Peer

@jbburt
Copy link
Member

jbburt commented Mar 7, 2021

Hi Peer,

I think this is a great idea but I'm not sure what is the cleanest way to implement it. The easiest thing to do would be to simply implement the methods you have suggested, i.e., the figure size and the font size (although, which font size specifically are you referring to? I'm also not exactly sure what you mean by colormap -- the facecolor of the scatter points?).

The problem more generally is that the two functions in the eval module already accept keyword arguments, which are then passed to the surrogate map constructors. I could implement a solution like that described in this stackoverflow post, but then we would need to specify exactly which matplotlib functions to pass keyword arguments to.

A third option would be to simply copy and paste the eval module's source code (which essentially just uses BrainSMASH to compute the variogram, and then uses matplotlib to plot it) and modify it in whichever way you want.

What do you think?

@PeerHerholz
Copy link
Author

PeerHerholz commented Mar 9, 2021

Hi @jbburt,

thx for the response, information and feedback.

I think, overall I was referring to variograms that are comparable to those in the corresponding publication, so that there would be an option to generate variograms that users could include in their publication (my current case, sorry for being selfish). Re fontsize: thus, definitely x and y labels, as well as the legend. If users want to include x and y ticks those also I would say. Re colormap/colors: yeah, scatter points and the line/std.

Ah true that, I totally forgot about the keywords for the eval module, sorry. That would indeed lead to confusion.

What about an option where computation and plots are two separate functions, i.e. one is the eval module/function and one is used to plot the outcomes of the former. E.g. base_fit and sampled_fit return emp_var, u0, surr_var which can then be used within a plotting function which takes kwargs? Via an argument in the eval module, e.g. plot_variogram (True or False) both the current option to automatically generate the default plot (True) or the output of emp_var, u0, surr_var to use within a dedicated plotting function (False) could be supported. That would also open up the possibility to say: "If you want to generate a different variogram use False and then use the outputs to create the plot yourself in the way you want.".

Sorry for the long and hopefully not too confusing response. WDYT?

@jbburt
Copy link
Member

jbburt commented Mar 10, 2021

Peer,

I've updated the eval module in BrainSMASH version 0.11.0 such that both functions (base_fit and sampled_fit) have a keyword argument return_data which, if True (but False by default), returns the three objects emp_var, u0, and surr_var. Please let me know if this satisfactorily addresses this issue for you!

Josh

@PeerHerholz
Copy link
Author

Hi Josh,

just tried it and works like a charm. Thx so much for the fast update and effort, highly appreciated!

Cheers, Peer

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