-
Notifications
You must be signed in to change notification settings - Fork 4
Updates for v0.2 #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
blinkseb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's really awesome thanks a million for that! I just noticed two small typos 👍
docs/in-depth/calling-momemta.md
Outdated
| momemta::Particle part2 { "part2", p2, 0 }; | ||
| std::vector<std::pair<double, double>> weights = weight.computeWeights({part1, part2}, met); | ||
| ``` | ||
| > *The `momemta::Particle` type consists of tree parts: a name (string) allowing to identify the particle in the configuration file, a 4-vector object, and a type (int). The type is not used by MoMEMta but can help the user configure the computation. It could be a PDG ID code, or any other code defined by the user.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
three instead of tree
docs/in-depth/configuration-file.md
Outdated
| BreitWignerGenerator.m_prop = { | ||
| ps_point = add_dimension(), | ||
| mass = parameter('my_mass'), -- Access the value through the "parameter" function, allowing it to be modified from the C++ code | ||
| width = parameter.my_width -- Access the value as a regular Lua table. It will NOT be possible to modify it later on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameters
d474066 to
780aefc
Compare
|
I've added a few things. Two obvious things are missing:
I think we have the minimum required for the next release? Unless I've missed something obvious. |
|
Many thanks! I'll try to read the documentation tomorrow see if I have some feedback! |
|
Ok, I can do a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!! Those additions are really awesome! I have a few comments nonetheless 😄
docs/in-depth/calling-momemta.md
Outdated
| In MoMEMta, the integration is defined by linking modules together in a Lua script. MoMEMta is shipped with a set of modules covering the most common needs and whose parameters, inputs and outputs are documented [here](https://momemta.github.io/MoMEMta/dev/group__modules.html). For more information on how to write the configuration file, see [here](configuration-file.md). | ||
|
|
||
| > *A module is just a C++ class deriving from the `Module` virtual class. How to write a new module and make it available for the calculation is described (here).* | ||
| > *A module is just a C++ class deriving from the `Module` virtual class.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can probably be a !!! note instead of a quote
docs/in-depth/calling-momemta.md
Outdated
| At this point, the user might wish to modify some parameters defined in the file from the code itself: | ||
| At this point, the user might wish to modify some parameters defined in the file from the code itself (see also [here](parameters)): | ||
| ```cpp | ||
| configuration.getGlobalParameters().set("top_mass", 173.); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configuration is not defined here, it should be my_reader
docs/in-depth/calling-momemta.md
Outdated
| ``` | ||
| > *MoMEMta is instantiated using a `Configuration` object, describing a fixed configuration: parameters cannot be modified at this stage. Parameters can be changed in the ConfigurationReader, and a new `MoMEMta` instance must then be constructed by calling `ConfigurationReader::freeze()` again.* | ||
| > *MoMEMta is instantiated using a `Configuration` object, describing a frozen configuration: parameters cannot be modified at this stage, the execution flow of the computation is fully fixed. Parameters can however be changed in the ConfigurationReader, and a new `MoMEMta` instance must then be constructed by calling `ConfigurationReader::freeze()` again.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe here also a !!! note is better than a quote?
docs/in-depth/calling-momemta.md
Outdated
|
|
||
| !!! note "" | ||
| The LorentzVectors are expected to be expressed in the `PxPyPzE<double>` basis | ||
| > *The `momemta::Particle` type consists of three parts: a name (string) allowing to identify the particle in the configuration file, a 4-vector object, and a type (int). The type is not used by MoMEMta but can help the user configure the computation. It could be a PDG ID code, or any other code defined by the user.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type part can be a bit misleading. There's currently no way to use it, even in the configuration file. It'll be in the future tho. And maybe it can be a !!! note too 😬
docs/in-depth/calling-momemta.md
Outdated
| MoMEMta provides the useful typedef `LorentzVector` for Lorentz Vectors in the file `momemta/Types.h` | ||
|
|
||
| `computeWeights()` returns a vector of pairs (weight, uncertainty) (at the moment, this vector only has one entry) which can then be stored by the user. | ||
| !!! note |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A !!! warning may be more suitable in this situation.
| ```Lua | ||
| P4Printer.printer1 = { input = "tf_part1::output" } | ||
| P4Printer.printer2 = { input = l_part1.gen_p4 } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is really nice, thanks, but think we should do things the way around: you first define inputs in the configuration file, and then pass them to the computeWeights function. It's mandatory to defines inputs, you cannot longer directly use hardcoded input tags. I'd also explicitly write that you access the reco input tag using the reco_p4 variable: currently, it's only written in the lua snippet, not in the text.
Maybe it's worth specify also that gen_p4 points by default to the same value that reco_p4: that's helpful if you don't need / want transfer function on the input, so that reco and gen quantities are the same thing.
docs/in-depth/configuration-file.md
Outdated
| Finally, the missing transverse energy 4-vector (passed optionally to `computeWeights()`) can either be accessed directly through the `met::p4` input tag, or declared as a Lua object as above: | ||
| ```Lua | ||
| local met = declare_input("met") | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not test, but I'm prettry sure this won't work. The met input is always declared, you don't need to do anything to access it.
| BreitWignerGenerator.m_prop = { | ||
| ps_point = add_dimension(), | ||
| mass = parameter('my_mass'), -- Access the value through the "parameter" function, allowing it to be modified from the C++ code | ||
| width = parameters.my_width -- Access the value as a regular Lua table. It will NOT be possible to modify it later on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be worth IMO to add a !!! danger block below insisting on this fact: you have to pass through the parameter function if you want the parameter to be editable
docs/in-depth/parameters.md
Outdated
| } | ||
| ``` | ||
| Four different integration algorithms are available, and each has several parameters than can be tweaked to adjust their behaviour, tweak the precision of the calculations, ... | ||
| The integration parameters can be changed from the C++ in a manner similar to the "global" parameters (previous section), using the method `ConfigurationReader::getCubaConfiguration()`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please link to in-depth/parameters/#defining-parameters
docs/in-depth/parameters.md
Outdated
| | `seed` | integers | `0` | | ||
| | `min_eval` | integers | `0` | | ||
| | `max_eval` | integers | `500000` | | ||
| | `grid_file` | paths | `` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO either put nothing as default value or something like <empty>
|
Many thanks for the comments, I've updated the PR! |
docs/in-depth/configuration-file.md
Outdated
|
|
||
| In C++, that input's 4-vector can then be passed through the `computeWeights()` function (see [Calling MoMEMta](calling-momemta)) by defining a `momemta::Particle` object whose name attribute is the same as the one used in the configuration, e.g. `momemta::Particle m_part1 { "part1", p1, 0 }`. | ||
|
|
||
| The Lua object `l_part1` contains the input tag needed to access the particle's 4-vector. For instance, defining a transfer function on the energy of `part1` can be done as: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe adding something like via the ``reco_p4`` attribute at the end of the first sentence?
|
Thanks a lot it's awesome! |
|
Updated again, addressing your last comment. I've also added a new page to document the phase-space parametrisations. It's still very much WIP, and could be skipped if we want to release earlier. |
|
Since the new page added by @swertz is not complete yet, I'd propose to remove it from this PR and PR it later on when it's done, so we can merge this. Any objections? |
|
I removed the page and opened #12 instead. This is ready to be merged, thanks a lot! |
I've taken some time to update the website for the coming second release.
This is not final since there might be still some changes, but don't hesitate if you already have some comments.
I've added @BrieucF to our team since he's contributed to the blocks!