Skip to content

mcviewtest: spawn the new mcplotdiff-html plotter#2568

Open
willend wants to merge 20 commits into
mainfrom
mcviewtest-spawn-diff-plotter
Open

mcviewtest: spawn the new mcplotdiff-html plotter#2568
willend wants to merge 20 commits into
mainfrom
mcviewtest-spawn-diff-plotter

Conversation

@willend

@willend willend commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Free-form text area

Please describe what your PR is adding in terms of features or bugfixes:


Declaration of use of AI-tools

  • Please add a checkmark here if you used AI-tools during the work for this contribution
  • Furter, please describe how / where and for what the tools were used:

Claude, prompt:

"Now I'd like you to focus on the mcviewtest tool which is part of tools/Python/mctest/.

The mctest tool can generate a set of test data for McStas, based on %Example lines in the instr files. As part of this, it calls the mcplot-html plotter per test result.

The folder structure from an mctest can be found at e.g. http://new-nightly.mcstas.org/todays-datafiles/mcstas-3.99.99_mpi_x_20_1e7_Linux_20260709_0902_23/

Afterwards, mcviewtest is called to generate an overview table - including columns that correspond e.g. to CPU and GPU execution - or different operating systems, see the html file at http://new-nightly.mcstas.org/todays-datafiles/2026-07-09_output.html (spans the above mctest output + 2 other mctest's.

Consider the first column in the html file the 'reference' test. Comparing each of the OTHER columns to the first one, I'd like you to add 'difference' cells corresponding to a mcplotdiff-html comparion between the dataset outputs, e.g. McStas_8GPU_openacc_mpi_x_7_1e7_Linux_20260709_0001_06_1e7/BNL_H8/1/ and mcstas-3.99.99_mpi_x_20_1e7_Linux_20260709_0902_23/BNL_H8/1/ - adding a link the the mcviewtest template."

Various local adaptation added


Development OS / boundary conditions

Please describe what OS you developed and tested your additions on, and if any special dependencies are required:


PR Checklist for contributing to McStas/McXtrace

For a coherent and useful contribution to McStas/McXtrace, please fill in relevant parts of the checklist:

  • My contribution contains something else

    • Explanation is added in free form text above or below the checklist

@willend
willend marked this pull request as draft July 9, 2026 20:23
@willend

willend commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

A first demo of what this can do is available through http://new-nightly.mcstas.org/demo-diffplots/2026-07-09_output.html. Where possible (simulation worked and produced ascii output), the other datasets are compared against the left-most column (“ref”).

While this still has rough edges (e.g. very long legends on links etc.) I think it will be very useful for instantly getting a good idea of what is up when a test starts to fail numerically.

@willend

willend commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

With the latest few commits (at 5ac3e2c) the output got a couple of stylistic changes.

Overview:
Screenshot 2026-07-10 at 11 03 06
(DIFF separated from rest of cell with a <hr>)

Diffplot:
Screenshot 2026-07-10 at 11 03 13
(Datasets labeled ‘A’ and ‘B’ in header, referral to these in link labels)

@willend

willend commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

After last commit, individual ‘monitor’ plots look like this:
Screenshot 2026-07-10 at 11 18 40
Screenshot 2026-07-10 at 11 18 29
(with link to the underlying ascii datafiles)

where as the ‘diff-monitor’ plots look like this:
Screenshot 2026-07-10 at 11 19 46
Screenshot 2026-07-10 at 11 19 56
(the ascii data do not exist in ‘physical form’)

@willend
willend marked this pull request as ready for review July 10, 2026 09:20
@willend willend changed the title DRAFT: mcviewtest: spawn the new mcplotdiff-html plotter mcviewtest: spawn the new mcplotdiff-html plotter Jul 10, 2026
@willend
willend requested a review from mads-bertelsen July 10, 2026 09:31
@willend

willend commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Here is a link to the output from mcstas-conda-testsuite as run from the PR bramch
http://new-nightly.mcstas.org/combined/combined_output.html

@mads-bertelsen mads-bertelsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great new capability! Only critical issue is the syntax issue in the generated html that doesn't close the span but instead opens a new one.

Comment on lines +165 to +169
# Only the Python tooling (mcviewtest, mcplotdiff-html, mcplot-html) is
# actually needed here - the diff/report generation is platform-agnostic
# regardless of which OS originally produced the monitor data - but we
# reuse the exact same install path as the build job for consistency,
# rather than maintaining a second, lighter-weight install mechanism.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay for now but could obviously be improved! Maybe we should make an issue on this to remember. Maybe its not a significant amount of time?


outfile.write(f"<a href='{b_log}' target=_blank>[ B (log) ]</a>\n")
if a_lin or a_log or b_lin or b_log:
outfile.write("<span class='origlinks'>\n")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be closing the span, instead it starts a new one

outfile.write("</head><body>\n")
outfile.write(f"<h1>Difference plots: {label_a} &minus; {label_b}</h1>\n")
outfile.write("<h1>Difference plots:</h1>\n")
outfile.write(f"<strong><ol type=\"A\"><li>{label_a} vs.<li>{label_b}</ol></strong>")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is different from the previewed webpage in the PR, it lacks the closing parts of the

  • so it doesn't seem to be valid html. Why use a list within an ordered list to show the heading?

  • ncols = len(lines_block[0].split())
    if ncols == 0:
    raise ValueError('empty first line in data block')
    flat = np.fromstring(' '.join(lines_block), dtype=float, sep=' ')

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This can create quite a big object in memory before its passed to numpy that makes it into floats. It could be done in parts to avoid having double the text size in memory at once, for example

    flat = np.concatenate([np.fromstring(line, dtype=float, sep=' ') for line in lines_block])

    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

    Successfully merging this pull request may close these issues.

    2 participants