Skip to content

Gridpack improvements: fewer files, compression, output directories - #250

Merged
oliviermattelaer merged 15 commits into
mainfrom
feat-improve-gridpacks
Oct 16, 2026
Merged

oliviermattelaer merged 15 commits into
mainfrom
feat-improve-gridpacks

Conversation

@theoheimel

@theoheimel theoheimel commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

This PR aims to make CMS happy. It reduces the number of small files in MG7 gridpacks by a lot, and allows flexible output paths for temporary and final files such that gridpacks can be read-only.

Changes

  • [gridpack] compress (default false): pack the gridpack into gridpack.tar.gz and remove the uncompressed directory.

  • No include folder: the built madspace copied into a gridpack no longer ships its include/ directory.

  • temp_output_dir / output_dir ([gridpack] run card options, and --temp_output_dir / --output_dir for the gridpack's bin/generate_events):

    • output_dir receives the final files, and no Events/<run> folder is created when it is set.
    • temp_output_dir receives the temporary per-channel npy files.
    • Defaults ("") keep the current behaviour.
    • Also works in regular runs through the run card. The shipped gridpack card has both blanked.
    • Relative command-line paths refer to the directory the gridpack is called from.
  • data/globals.tar replaces the data/globals/ directory of small npy files.

    • New TarWriter / TarReader (madspace/driver/tar.hpp) write and read standard POSIX ustar archives, readable by tar, Python's tarfile and numpy.
    • save_tensor / load_tensor gained overloads for tar entries. The npy format is unchanged.
    • Removed UB in the npy header code (union type punning, reinterpret_cast).
    • Corrected the Tensor::is_contiguous doc comment (storage is column-major).
  • data/channels.json replaces the per-channel data/channels/*.json. ChannelEventGenerator gained to_json() and load_json().

  • Source hash check: a madspace source hash mismatch between the gridpack and the installed madspace is now an error. bin/generate_events --ignore_source_hash runs anyway and still prints the warning.

  • [gridpack] run_mode (regular, minimal, fix_max_weight; default minimal; only used when save_gridpack is set):

    • regular: a normal run with events, then the gridpack is stored.
    • minimal: no events. With MadNIS the gridpack is stored right after the training, with VEGAS once the grid of every channel has converged (new EventGenerator::optimize()).
    • fix_max_weight: no events. Every channel runs until freeze_max_weight_after unweighted events (new EventGenerator::fix_max_weights()). Each channel's max weight is stored in the gridpack and kept fixed, and its integral is stored and combined (inverse-variance weighted) with the on-the-fly estimate to share the events between channels. Caveat: a low-efficiency channel can make this mode slow.
    • Both non-regular modes skip plots, post-processing and the selected tools, since there are no events.
  • Docs: docs/source/gridpacks.rst describes the new layout, compress, the output directories and run_mode.

Compatibility

Gridpacks created before this change (with data/globals/ and data/channels/) cannot be run by the new bin/generate_events.

Testing

  • New madspace/tests/test_tar.py: round trip, and reading/extracting with system tar, tarfile and numpy.
  • The four test_*reproducib*_mg7 tests pass (with MG7_REPRO_EVENTS=2000), including the MadNIS and VEGAS gridpack ones with the new minimal default.
  • Manual runs of e+ e- > mu+ mu- gridpacks in each run_mode, with the default settings and with the new options.
  • Not run: the rest of the madspace test suite, GPU backends.

@theoheimel
theoheimel marked this pull request as ready for review October 12, 2026 09:37
@oliviermattelaer

Copy link
Copy Markdown
Contributor
  • Note that this is not pyton3.11 compliant -> One f-string nests double quotes, which only Python 3.12+ accepts, but madspace allows 3.11. (in launch.py)

  • check this one: Claude says that bin/generate_events crashes when the madgraph package can be imported.
    That is the setup the script tries first. RunCardMG7.get takes only one argument, so run_card.get("gridpack", {}) raises a TypeError. The PR’s tests probably passed because madgraph couldn’t be imported there, so the script used its plain-TOML fallback.

  • Not sure if this one is this PR specific (but important to fix): fix_max_weight gridpacks can report a cross section that is too low. When a run asks for few events, any channel whose share rounds to 0 events is never sampled. Its cross section is then missing from the total and from the event weights, and the reported error comes out as NaN.

  • for temp_output_dir , it could be nice to allow for True which will use a temporary directory (using tempfile.mkdtemp() to avoid between run collision

  • one Claude suggestion is to ensure that the zipping is done with multi-thread (maybe not critical)

  • In GenerateMode::optimize_only, VEGAS jobs are still dispatched with .unweight = true, so every optimization batch is unweighted and written to the temp npy files, even though this mode keeps no events.

@theoheimel

Copy link
Copy Markdown
Contributor Author
  • check this one: Claude says that bin/generate_events crashes when the madgraph package can be imported.
    That is the setup the script tries first. RunCardMG7.get takes only one argument, so run_card.get("gridpack", {}) raises a TypeError. The PR’s tests probably passed because madgraph couldn’t be imported there, so the script used its plain-TOML fallback.

@oliviermattelaer We should consider dropping that import of the madgraph package altogether, since the goal of gridpacks is to be self-contained. Or do we need it for anything?

@oliviermattelaer

Copy link
Copy Markdown
Contributor

It actually does work without it, but has issue if madgraph is linked to the gripack:

# lines 98–110
    run_card_path = os.path.join("Cards", "grid_run_card.toml")
    try:
        from madgraph.various.banner import RunCardMG7      # line 103
        run_card = RunCardMG7(run_card_path)
    except ImportError:
        with open(run_card_path, "rb") as f:
            run_card = tomllib.load(f)                      # plain-dict fallback
    run_args = run_card["run"]
    gen_args = run_card["generation"]
    grid_args = run_card.get("gridpack", {})                # line 110 ← new in PR 250

The try: from madgraph... already exists on main (line 99), so it isn’t new. What’s new is the run_card.get("gridpack", {}) call:

  • madgraph can’t be imported: run_card is a plain dict, and .get with a default works.
  • madgraph can be imported (e.g. MG on your PYTHONPATH): run_card is a RunCardMG7, whose get = getitem takes only one argument, so the call raises a TypeError.

The real question is how do we support madgraph to be importable in general.
For MG5, we do not recommend to add MG5 into pythonpath and therefore, we conseder havint it in path a way to overwitre the local python code (usefull to development). If we do not change that then the solution is to either fully drop the RunCardMG7 or to allow for a local import (I would prefer that).

If we advise to have MG in the PythonPath then we need a full change of the code and decide how to handle mismatch configuration.

@theoheimel

Copy link
Copy Markdown
Contributor Author

I'll drop it then. All run-card settings for gridpacks can be overwritten through command line arguments, so overwriting local code should not be necessary because the only place where the madgraph package is used is reading the run card. This simplifies things and avoids such issues in the future.

…in optimize_only, temp_output_dir auto, pigz compression, drop RunCardMG7 from gridpack, py3.11 f-string
@theoheimel

Copy link
Copy Markdown
Contributor Author

@oliviermattelaer I implemented you feedback. So this can be merged once tests pass.

@oliviermattelaer oliviermattelaer 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.

Thanks Theo

@oliviermattelaer
oliviermattelaer merged commit feee1d8 into main Oct 16, 2026
183 checks passed
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.

3 participants