NNPDF / nnpdf

An open-source machine learning framework for global analyses of parton distributions.
https://docs.nnpdf.science/
GNU General Public License v3.0
30 stars 6 forks source link

Divide certain plots into multiple columns in vp reports #268

Closed voisey closed 4 years ago

voisey commented 6 years ago

In the standard validphys report certain plots should be divided into multiple (two?) columns to make it easier to read through plots and compare them. This would be especially useful for the PDF plots, distance plots, positivity plots, and data/theory comparison plots.

Zaharid commented 6 years ago

So there are various things here.

First of all, I suppose this would apply to functions tagged with @figuregen, so the ones that output a bunch of figures. The relevant code is here:

https://github.com/NNPDF/reportengine/blob/master/src/reportengine/figure.py

I suppose one could modify the way savefiglist works to print out the figures differently

Then, AFAICT there is no way to do this with markdown in a way that keeps the captions:

https://stackoverflow.com/questions/15367332/pandoc-image-alignment

so we would probably need to output custom html. We already do that for tables, so it is fine, albeit slightly annoying: in principle the output of the reports could easily be converted to e.g. LaTeX but adding custom HTML interferes with that. I think for this to happen, at minimum someone has to come up with a reasonable piece of HTML that allows you to put two images together. For referece, this is what pandoc does for a single figure:


<div class="three-fourths column markdown-body">
<figure>
<img src="figures/default_theory0_plot_expplusthcorrmat_heatmap_custom.png" alt=".png .pdf" /><figcaption><a href="figures/default_theory0_plot_expplusthcorrmat_heatmap_custom.png">.png</a> <a href="figures/default_theory0_plot_expplusthcorrmat_heatmap_custom.pdf">.pdf</a></figcaption>
</figure>
RosalynLP commented 5 years ago

@Zaharid I have been looking at this and had a couple of questions: in reportengine/figure.py the Figureclass has the property as_markdown which returns something similar to what is in that pandoc stack overflow link. Would a first step to be to return two images together e.g.

![](image1.kpg)\![](image2.jpg)?

or do you mean that we should abandon markdown altogether in favour of html? In that case I don't understand a) where the html for reportengine is generated and b) whereabouts I need to look to put some html for this particular function.

Is there any documentation for reportengine anywhere?

Cheers, Rosalyn

Zaharid commented 5 years ago

On Fri, May 31, 2019 at 4:58 PM RosalynLP notifications@github.com wrote:

@Zaharid https://github.com/Zaharid I have been looking at this and had a couple of questions: in reportengine/figure.py the Figure class has the property as_markdown which returns something similar to what is in that pandoc stack overflow link. Would a first step to be to return two images together e.g.

! ?

I don't believe this does what you want because I tried it at some point and it didn't work (I think it was because of the captions).

or do you mean that we should abandon markdown altogether in favour of html? In that case I don't understand a) where the html for reportengine is generated and b) whereabouts I need to look to put some html for this particular function.

Is there any documentation for reportengine anywhere?

Markdown in general and the reports in particular work so that the html code you write in a markdown file is outputted verbatim. The final piece of html is generated by taking the markdown text (which gets saved to a file) and running pandoc on it (that is what the report action does).

A simple way to experiment would be to write html in template_text without any particular validphys actions.

Cheers, Rosalyn

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NNPDF/nnpdf/issues/268?email_source=notifications&email_token=ABLJWUUJ7CTHF3TSTSY5UOLPYFDKJA5CNFSM4FWGHXZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWVUK3A#issuecomment-497763692, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLJWUWBCW3X32QDG462X5DPYFDKJANCNFSM4FWGHXZQ .

RosalynLP commented 5 years ago

@Zaharid OK cool, thanks :)

RosalynLP commented 5 years ago

@Zaharid so I wrote in a piece of html which produces three images with separate captions:

https://vp.nnpdf.science/YLxZxfZnQdSxAc7LIr0fyA==/

what is the end-goal exactly, I am still confused about what I am trying to produce and where we would want this html to be written eventually - presumably not in template text in the end?

Zaharid commented 5 years ago

@RosalynLP Well, you, or @voisey who opened the issue tell me. Like I said above, a simple way to proceed would be to change the way @figuregen works to output some html with the desired properties instead of the list of markdown figures.

RosalynLP commented 5 years ago

Doesn't savefiglist only generate the list of figures to be saved on file? I don't see where the markdown comes into it, unless it's the as_markdown property, but I don't understand where this is used. Should it be as_markdown we'd need to edit?

Zaharid commented 5 years ago

It returns a list, wich reportengine prints as the concatenation of the elements (in the top level as_markdown function). It could instead return a string with the HTML or better an object with the as_markdown method returning the string.

On Thu, 6 Jun 2019, 15:13 RosalynLP, notifications@github.com wrote:

Doesn't savefiglist only generate the list of figures to be saved on file? I don't see where the markdown comes into it, unless it's the as_markdown property, but I don't understand where this is used. Should it be as_markdown we'd need to edit?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NNPDF/nnpdf/issues/268?email_source=notifications&email_token=ABLJWUTNAMNNN7M7ENIM2JLPZELQ7A5CNFSM4FWGHXZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXC7FDI#issuecomment-499511949, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLJWUS4O5XPMS7RP4NOBTLPZELQ7ANCNFSM4FWGHXZQ .