Updates to impact plots and code to run likelihood scans #73
Conversation
rbarrue
left a comment
rbarrue
left a comment
There was a problem hiding this comment.
Hi Sergio, the PR looks great!
I just have some comments asking for additional documentation.
| return self | ||
|
|
||
| def set(self, value): | ||
| if self.isFrozen: |
There was a problem hiding this comment.
Hi Sergio, this new feature looks great. I would assume this is for binned multidimensional fits ?
There were some things that I did not fully understand at first glance (such as what I asked above), which brings me to this: could you add a docstring at the beginning so that one can get a quick overview of what the script does and what is each of its parameters?
Also could you add an example config so that one can understand how to define additional fit variables and binning ?
| p = argparse.ArgumentParser(description="TFMC training (YAML-driven)") | ||
| p.add_argument("config", help="Path to global YAML config") | ||
| p.add_argument("--rotate", action="store", default=None, help="Point to a rotate JSON") | ||
| p.add_argument("--algo", default="grid", choices=["grid"]) |
There was a problem hiding this comment.
I think it would make sense to add some additional description to this argument, let me know if you disagree.
Also, what would be other possible options beyond grid ?
| p.add_argument("config", help="Path to global YAML config") | ||
| p.add_argument("--rotate", action="store", default=None, help="Point to a rotate JSON") | ||
| p.add_argument("--algo", default="grid", choices=["grid"]) | ||
| p.add_argument("--freezeParameters", default="", help="Parameters to freeze. Otherwise they float") |
There was a problem hiding this comment.
If can I just make a quick last request: for cases where the parser expects a single string with comma-separated values (like the one in this line), could you just put "(string with comma-separated values)" or smth like that ?
schoef
left a comment
schoef
left a comment
There was a problem hiding this comment.
Can you put a usage example of slurm_utils? At some point we have to move stuff to subdirectories. Otherwise looks good.
3 participants
Likelihood