MCPL_output’s “mccode_cmd_line” blob stores the default input values

[MilanKlausz]

The MCPL_output component (all versions, including the McStas, McXtrace, and _noacc) stores a blob in the MCPL header with the key named "mccode_cmd_line". I expected it to store the command used to invoked the McStas simulation (with the input parameters), but it actually stores a line to invoke the .out file with the DEFAULT values for all input parameter for the instrument. e.g., ./loki_master_model.out collen=3.0 l_min=3.0 l_max=11.5 ...

For me, this is:

1. Confusing because of the default values (rather than the actual values used for the simulation)
2. Redundant because the other blob saved (mccode_instr_file) contains the whole instrument file, which includes default input parameters at the beginning ((I haven't checked what happens when the instrument file includes an another one with %include))

Checking the code history, I found that the original implementation (commit message:“command line pars are now packed in a binary header blob”) did in fact store the actual values, but later it was changed to the default values with the message: “fix header blob bug which might prevent merges”.

I am aware of that multiple MCPL files can only be merged if the headers (including the blobs) are the same, but I don't see any reasonable case when the merge could fail for a single simulation if the actually used values of the input parameters would be stored. (If it would fail because the MCPL files to be merged are from simulations with different settings, then the --forcemerge MCPL option could/should be used.)

I could easily be wrong, or miss a specific use case, so could you someone explain why it is the way it is? (Maybe @ebknudsen, who authored both the original commit and the modification?)

[ebknudsen]

It's been while but the intention of the original implementation was indeed to store the parameters that were used for the specific run. IIRC the reason for the fix was exactly to allow merging (something that was requested by several users at the time) at a point in time where --forcemerge was not available in mcpltools. As --forcemerge now exists it would IMHO make sense to revert the "fix". I totally agree that the actual parameters are what is useful - less so the default ones.

[willend]

Thanks @MilanKlausz for pointing this out and @ebknudsen for making some sort of sense why things are as they are.

Since then, @tkittel (backed by popular demand) implemented --forcemerge so consider the proposed (and very sensible) behaviour of MCPL_output-variants on the TODO for next release(s).

[willend]

I believe 840f4c6e6 brings this in order.

[g5t]

@MilanKlausz said in the initial issue description:

I don't see any reasonable case when the merge could fail for a single simulation if the actually used values of the input parameters would be stored. (If it would fail because the MCPL files to be merged are from simulations with different settings, then the --forcemerge MCPL option could/should be used.)

I'm working on a tool that, among other things, repeats simulations until a target number of neutrons are accumulated in an MCPL file. To do this, it needs to specify the output MCPL filename as part of the command line arguments to the simulation, so each run produces a different file which can be combined after the fact.

Previously I could make use of mcpltool --merge but now that the actual command line arguments are included in the MCPL file that fails somewhat cryptically.

I'll change my tool, either to recycle the same output filename between runs, or to make use of mcpltool --forcemerge; I just wanted to provide some context for the relevant XCKD linked above.

@willend Can this issue be closed as it seems to be completed?

[tkittel]

If anyone comes up with a proposal for how MCPL could better accomodate these kinds of issues, please open a feature request in the mcpl issue tracker. So far I haven't been able to come up with something that isn't somehow worse than the current situation :-)