Skip to content

feat: Unified binary (CLI)#917

Open
Charlotte-Knight wants to merge 26 commits into
developfrom
unified_binary
Open

feat: Unified binary (CLI)#917
Charlotte-Knight wants to merge 26 commits into
developfrom
unified_binary

Conversation

@Charlotte-Knight

@Charlotte-Knight Charlotte-Knight commented May 28, 2026

Copy link
Copy Markdown
Contributor

User Interface

This PR introduces a single binary through which to run MaCh3 executables. In other words, it introduces a unified CLI.

For example, one can run mach3 --help and get:

Usage:  [--help] [--version] {diag,penterm,process,tutorial}

Optional arguments:
  -h, --help     shows help message and exits
  -v, --version  prints version information and exits

Subcommands:
  diag
  penterm
  process       Main exectable responsible for different types of MCMC processing like drawing posteriors, triangle plots etc.

and then run ProcessMCMC with:

mach3 process config.yaml chain.root

So far, the following executables have been implemented:

  • DiagMCMC
  • ProcessMCMC
  • GetPenaltyTerm

And the functionality exists for experiments to add their own executables via plugins. An example of this is included in MaCh3Tutorial via this PR: mach3-software/MaCh3Tutorial#252. There, you can run the MCMCTutorial executable like:

mach3 tutorial TutorialConfigs/FitterConfig.yaml --MCMCSteps 100000

The previous name for the commands can still be used like:

ProcessMCMC config.yaml chain.root

and the user will be presented with a deprecation warning like:

[2026-06-01 14:04:37.661] [warning] [ProcessMCMC.cpp:5] Deprecation Warning: Use of the standalone executable will be deprecated in future releases.
[2026-06-01 14:04:37.661] [warning] [ProcessMCMC.cpp:6]                    : you can use 'mach3 process' as a direct replacement instead.

Feedback

We're open to suggestions about the naming of the commands, e.g. whether mach3 process makes sense or maybe mach3 processmcmc etc. And anything other comments are welcome of course.

Future Plans

Assuming that this PR is merged, we imagine that this executable will start to gain traction with some users, and hopefully, we can gather feedback. In time, we can implement that feedback and eventually implement the rest of the executables and remove the old interface.

Implementation

TODO: Comments about implementation for the other devs. For example, all of the executables going under the CLI/API directory.

Subcommand map

Old Executable Name CLI Subcommand
ProcessMCMC process
DiagMCMC diag
GetPenaltyTerm penterm

@KSkwarczynski KSkwarczynski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have plenty of comments, although majority are nitpicky and style.

My main comment is lack of documentation or short explanation why X was done like this etc.

Doesn't have to be encyplopedia.

Comment thread CLI/API/CMakeLists.txt
Comment thread cli/modules/DiagMCMCModule.cpp Outdated
Comment thread cli/api/argparse.hpp
Comment thread cli/api/plugin.hpp Outdated
Comment thread CLI/Modules/DiagMCMCModule.cpp
Comment thread CLI/Modules/ProcessMCMCModule.cpp
Comment thread CLI/MaCh3Program.cpp
Comment on lines +39 to +40
std::cerr << "Cannot determine HOME directory\n";
return;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is std::cerr etc used here because it is part of CLI not actual executable?

Comment thread cli/api/argparse.hpp
Comment on lines +1 to +3
#pragma once
#include <argparse/argparse.hpp>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we hide this under MaCh3_Safe_Include_Start ?

MaCh3/Manager/Core.h

Lines 122 to 125 in d491feb

/// @brief KS: Avoiding warning checking for headers
/// @details Many external files don't strictly adhere to rigorous C++ standards.
/// Inline functions in such headers may cause errors when compiled with strict MaCh3 compiler flags.
/// @warning Use this for any external header file to avoid warnings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't understand this comment, can you ellaborate?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So mach3 is compiled with very rigorous set compilation flags (Werror, Wall etc.)
Sometimes header files have actual implementation and they "external" headers are being compiled with MaCh3 flags.

Which causes compilation errors.

By doing

_MaCh3_Safe_Include_Start_ //{
#include "argparse/argparse.hpp"
_MaCh3_Safe_Include_End_ //}

we says compiler to ignore some warnigns/errors for a given header.
Which allows to avoid having annoing errors.

Comment thread CLI/mach3.cpp Outdated
Comment thread CMakeLists.txt Outdated
@KSkwarczynski

Copy link
Copy Markdown
Member

I also think this need to be updated

INPUT = . \
Doc/MaCh3Web/mainpage.md \
Doc/MaCh3Web/Definitions.dox \
Doc/mach3logo.png \
Manager \
Splines \
Samples \
NuDock \
Fitters \
Diagnostics \
Plotting \
Plotting/plottingUtils \
Parameters \
python \
Doc/User-Guide/

@Charlotte-Knight Charlotte-Knight marked this pull request as ready for review June 19, 2026 14:51
@KSkwarczynski

Copy link
Copy Markdown
Member

There are some merge conflicts.

Is it possible to use
///
instead of

/**
 *
 */

for a doxygen to be consistent with rest of code base?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants