Modifications of nT simulation context #602

terliuk · 2021-07-26T19:01:35Z

What does the code in this PR do / what does it improve?

This PR modifies the nT simulation context for more flexibility.

Can you briefly describe how it works?

Main difference from previous one is an option of making fully divergent properties for simulations (wfsim instructions -> raw_records + truth) and processing (raw_records -> peaklets -> events etc).

Can you give a minimal working example (or illustrate with a figure)?

The example of different context options and its usage can be found in this notebook.

github-actions

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

pep8

straxen/contexts.py|251 col 78| W291 trailing whitespace
straxen/contexts.py|253 col 1| W293 blank line contains whitespace
straxen/contexts.py|256 col 1| RST201 Block quote ends without a blank line; unexpected unindent.
straxen/contexts.py|256 col 81| W291 trailing whitespace
straxen/contexts.py|257 col 37| W291 trailing whitespace
straxen/contexts.py|258 col 82| W291 trailing whitespace
straxen/contexts.py|262 col 73| W291 trailing whitespace
straxen/contexts.py|264 col 75| W291 trailing whitespace
straxen/contexts.py|265 col 79| W291 trailing whitespace
straxen/contexts.py|267 col 78| W291 trailing whitespace
straxen/contexts.py|268 col 38| W291 trailing whitespace
straxen/contexts.py|269 col 79| W291 trailing whitespace
straxen/contexts.py|270 col 34| W291 trailing whitespace
straxen/contexts.py|277 col 1| W293 blank line contains whitespace
straxen/contexts.py|289 col 1| W293 blank line contains whitespace
straxen/contexts.py|292 col 1| W293 blank line contains whitespace
straxen/contexts.py|294 col 8| WPS508 Found incorrect not with compare usage
straxen/contexts.py|294 col 41| WPS508 Found incorrect not with compare usage
straxen/contexts.py|295 col 12| WPS508 Found incorrect not with compare usage
straxen/contexts.py|295 col 31| E225 missing whitespace around operator
straxen/contexts.py|299 col 39| WPS508 Found incorrect not with compare usage
straxen/contexts.py|300 col 23| E225 missing whitespace around operator
straxen/contexts.py|301 col 10| WPS508 Found incorrect not with compare usage
straxen/contexts.py|302 col 24| E225 missing whitespace around operator
straxen/contexts.py|305 col 1| W293 blank line contains whitespace
straxen/contexts.py|307 col 62| W291 trailing whitespace
straxen/contexts.py|308 col 1| W293 blank line contains whitespace
straxen/contexts.py|310 col 57| W291 trailing whitespace
straxen/contexts.py|311 col 59| W291 trailing whitespace
straxen/contexts.py|313 col 36| C408 Unnecessary dict call - rewrite as a literal.
straxen/contexts.py|316 col 53| E126 continuation line over-indented for hanging indent
straxen/contexts.py|316 col 67| W291 trailing whitespace
straxen/contexts.py|317 col 69| W291 trailing whitespace
straxen/contexts.py|318 col 78| C812 missing trailing comma
straxen/contexts.py|319 col 53| E123 closing bracket does not match indentation of opening bracket's line
straxen/contexts.py|321 col 1| W293 blank line contains whitespace
straxen/contexts.py|324 col 68| WPS204 Found overused expression: cmt_options[option]; used 5 > 4
straxen/contexts.py|325 col 1| W293 blank line contains whitespace
straxen/contexts.py|329 col 5| WPS337 Found multiline conditions
straxen/contexts.py|329 col 38| W504 line break after binary operator
straxen/contexts.py|329 col 40| W291 trailing whitespace
straxen/contexts.py|330 col 17| E127 continuation line over-indented for visual indent
straxen/contexts.py|335 col 60| E231 missing whitespace after ','
straxen/contexts.py|338 col 61| E231 missing whitespace after ','
straxen/contexts.py|338 col 85| W291 trailing whitespace
straxen/contexts.py|339 col 1| W293 blank line contains whitespace
straxen/contexts.py|340 col 85| W291 trailing whitespace
straxen/contexts.py|342 col 5| WPS528 Found implicit .items() usage
straxen/contexts.py|346 col 12| E713 test for membership should be 'not in'
straxen/contexts.py|346 col 12| WPS508 Found incorrect not with compare usage
straxen/contexts.py|348 col 31| E127 continuation line over-indented for visual indent
straxen/contexts.py|348 col 53| W291 trailing whitespace
straxen/contexts.py|349 col 9| WPS327 Found useless continue at the end of the loop
straxen/contexts.py|349 col 13| B007 Loop control variable 'fax_key' not used within the loop body. If this is intended, start the name with an underscore.
straxen/contexts.py|349 col 20| E231 missing whitespace after ','
straxen/contexts.py|350 col 23| E225 missing whitespace around operator
straxen/contexts.py|350 col 31| E701 multiple statements on one line (colon)
straxen/contexts.py|350 col 33| WPS220 Found too deep nesting: 32 > 20
straxen/contexts.py|352 col 51| WPS441 Found control variable used after block: fax_key
straxen/contexts.py|353 col 53| E126 continuation line over-indented for hanging indent
straxen/contexts.py|353 col 73| WPS121 Found usage of a variable marked as unused: _name_index
straxen/contexts.py|354 col 85| C812 missing trailing comma
straxen/contexts.py|355 col 62| E126 continuation line over-indented for hanging indent
straxen/contexts.py|356 col 9| WPS313 Found parenthesis immediately after a keyword
straxen/contexts.py|356 col 20| E231 missing whitespace after ','
straxen/contexts.py|356 col 28| E231 missing whitespace after ','
straxen/contexts.py|356 col 29| WPS121 Found usage of a variable marked as unused: _name_index
straxen/contexts.py|358 col 5| WPS528 Found implicit .items() usage
straxen/contexts.py|362 col 9| WPS121 Found usage of a variable marked as unused: _name_index
straxen/contexts.py|363 col 9| WPS221 Found line with high Jones Complexity: 16 > 14
straxen/contexts.py|363 col 50| WPS121 Found usage of a variable marked as unused: _name_index
straxen/contexts.py|363 col 101| E501 line too long (111 > 100 characters)
straxen/contexts.py|364 col 9| WPS313 Found parenthesis immediately after a keyword
straxen/contexts.py|364 col 13| WPS121 Found usage of a variable marked as unused: _name_index
straxen/contexts.py|367 col 1| W293 blank line contains whitespace
straxen/contexts.py|368 col 5| WPS313 Found parenthesis immediately after a keyword

straxen/contexts.py

github-actions · 2021-07-26T19:02:48Z

straxen/contexts.py

-                                                     electron_lifetime_liquid='elife_conf'),
-                       **kwargs):
+
+def xenonnt_simulation(


[pep8] _{reported by reviewdog 🐶}
WPS238 Found too many raises in a function: 4 > 3

I cannot even mark it but would you mind to stick to python code guidlnes here. Which means no space before and after the equality sign in the function arguments.

straxen/contexts.py

WenzDaniel

Here is a first bunch of style comments. Once committed, I will check the code itself.

WenzDaniel · 2021-07-27T15:29:56Z

straxen/contexts.py

-                                                     electron_lifetime_liquid='elife_conf'),
-                       **kwargs):
+
+def xenonnt_simulation(


I cannot even mark it but would you mind to stick to python code guidlnes here. Which means no space before and after the equality sign in the function arguments.

straxen/contexts.py

WenzDaniel · 2021-07-27T15:50:24Z

straxen/contexts.py

+    # doing sanity checks for cmt run ids for simulation and processing
+    if not (cmt_run_id_sim is None) and not (cmt_run_id_proc is None):
+        if not (cmt_run_id_sim==cmt_run_id_proc):
+            print("INFO : divergent CMT runs for simulation and processing")
+            print("    cmt_run_id_sim".ljust(25), cmt_run_id_sim)
+            print("    cmt_run_id_proc".ljust(25), cmt_run_id_proc)
+    elif (cmt_run_id_sim is None) and not (cmt_run_id_proc is None):
+        cmt_run_id_sim=cmt_run_id_proc
+    elif not (cmt_run_id_sim is None) and (cmt_run_id_proc is None):
+        cmt_run_id_proc=cmt_run_id_sim
+    else:
+        raise RuntimeError("Trying to set both cmt_run_id_sim and cmt_run_id_proc to None")


Suggested change

# doing sanity checks for cmt run ids for simulation and processing

if not (cmt_run_id_sim is None) and not (cmt_run_id_proc is None):

if not (cmt_run_id_sim==cmt_run_id_proc):

print("INFO : divergent CMT runs for simulation and processing")

print(" cmt_run_id_sim".ljust(25), cmt_run_id_sim)

print(" cmt_run_id_proc".ljust(25), cmt_run_id_proc)

elif (cmt_run_id_sim is None) and not (cmt_run_id_proc is None):

cmt_run_id_sim=cmt_run_id_proc

elif not (cmt_run_id_sim is None) and (cmt_run_id_proc is None):

cmt_run_id_proc=cmt_run_id_sim

else:

raise RuntimeError("Trying to set both cmt_run_id_sim and cmt_run_id_proc to None")

# doing sanity checks for cmt run ids for simulation and processing

if not (cmt_run_id_sim and cmt_run_id_proc ):

raise RuntimeError("Trying to set both cmt_run_id_sim and cmt_run_id_proc to None")

if (cmt_run_id_sim and cmt_run_id_proc ) and (cmt_run_id_sim!=cmt_run_id_proc):

warnings.warn(("INFO : divergent CMT runs for simulation and processing",

" cmt_run_id_sim".ljust(25), cmt_run_id_sim,

" cmt_run_id_proc".ljust(25), cmt_run_id_proc))

else:

cmt_id = cmt_run_id_sim or cmt_run_id_proc

cmt_run_id_sim = cmt_id

cmt_run_id_proc = cmt_id

This change you should test after committing.

I more or less change it in that direction, but i kept it as print and not warning, this is not intended to be the warning. But this opens an important point - do we have a proper logger with configurable verbosity level?

Yes strax and straxen have it both.

…lines

WenzDaniel

Looks fine, a little extensive and very special so we may want to consider to completely remove it from straxen at a later stage. I added a few comments. I do not understand the codefactor complains but hopefully they vanish when we trigger the tests again.

straxen/contexts.py

WenzDaniel · 2021-07-28T05:59:12Z

straxen/contexts.py

+            if overwrite_from_fax_file_sim:
+                st.config['fax_config_override_from_cmt'][fax_field] = (
+                         cmt_options[cmt_field][0] + '_constant',fax_config[fax_field])


This term is confusing to me. You are overwriting the cmt options which overwrite fax to keep the fax values? Would not it be easier in this case to simply not overwrite anything. I think you are changing the lineage here as you go from cmt_option_name to cmt_option_name_constant.

isn't it exactly the point? we track those parameters used for raw_record creation in lineage

straxen/contexts.py

WenzDaniel

I think it is fine now. A bit complex but should work.

WenzDaniel · 2021-07-29T09:51:38Z

@ershockley or @jorana one of you guys have to merge as I do not have any admin privileges.

JoranAngevaare · 2021-07-31T17:25:06Z

ok I did not look at it but merging by Daniels request, thanks Andrii!

* Modificatons of nT simulation context * Throwing some whitespae bones to the doggy * some changes to make Daniel a bit happier, but still leeping shorter lines * Changes to error and docstring * actually, there should be break here * Small modification in the loop * Empty line to redo codefactor * Hopefully, this is the correct way to fix codefactor complain Co-authored-by: Daniel Wenz <43881800+WenzDaniel@users.noreply.github.com>

Modificatons of nT simulation context

3098880

terliuk changed the title ~~Modificatons of nT simulation context~~ Modifications of nT simulation context Jul 26, 2021

github-actions bot reviewed Jul 26, 2021

View reviewed changes

Throwing some whitespae bones to the doggy

d6b7f33

terliuk marked this pull request as ready for review July 26, 2021 19:45

terliuk requested review from WenzDaniel and zhut19 July 27, 2021 07:23

WenzDaniel reviewed Jul 27, 2021

View reviewed changes

some changes to make Daniel a bit happier, but still leeping shorter …

f96a1dc

…lines

WenzDaniel reviewed Jul 28, 2021

View reviewed changes

terliuk and others added 6 commits July 28, 2021 03:14

Changes to error and docstring

18ae54f

actually, there should be break here

270a059

Small modification in the loop

ad50620

Empty line to redo codefactor

27863a1

Hopefully, this is the correct way to fix codefactor complain

c79b2fe

Merge branch 'master' into upd_nt_sim_context

4f57731

WenzDaniel approved these changes Jul 29, 2021

View reviewed changes

JoranAngevaare merged commit 4c0ff15 into master Jul 31, 2021

JoranAngevaare deleted the upd_nt_sim_context branch July 31, 2021 17:25

zhut19 mentioned this pull request Sep 13, 2021

Match straxen config with fax_config in certain parameters. XENONnT/WFSim#174

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modifications of nT simulation context #602

Modifications of nT simulation context #602

terliuk commented Jul 26, 2021

github-actions bot left a comment

github-actions bot Jul 26, 2021

WenzDaniel Jul 27, 2021 •

edited

Loading

WenzDaniel left a comment

WenzDaniel Jul 27, 2021 •

edited

Loading

WenzDaniel Jul 27, 2021

terliuk Jul 27, 2021

WenzDaniel Jul 28, 2021

WenzDaniel left a comment

WenzDaniel Jul 28, 2021

terliuk Jul 28, 2021

WenzDaniel left a comment

WenzDaniel commented Jul 29, 2021

JoranAngevaare commented Jul 31, 2021

Modifications of nT simulation context #602

Modifications of nT simulation context #602

Conversation

terliuk commented Jul 26, 2021

What does the code in this PR do / what does it improve?

Can you briefly describe how it works?

Can you give a minimal working example (or illustrate with a figure)?

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jul 26, 2021

Choose a reason for hiding this comment

WenzDaniel Jul 27, 2021 • edited Loading

Choose a reason for hiding this comment

WenzDaniel left a comment

Choose a reason for hiding this comment

WenzDaniel Jul 27, 2021 • edited Loading

Choose a reason for hiding this comment

WenzDaniel Jul 27, 2021

Choose a reason for hiding this comment

terliuk Jul 27, 2021

Choose a reason for hiding this comment

WenzDaniel Jul 28, 2021

Choose a reason for hiding this comment

WenzDaniel left a comment

Choose a reason for hiding this comment

WenzDaniel Jul 28, 2021

Choose a reason for hiding this comment

terliuk Jul 28, 2021

Choose a reason for hiding this comment

WenzDaniel left a comment

Choose a reason for hiding this comment

WenzDaniel commented Jul 29, 2021

JoranAngevaare commented Jul 31, 2021

WenzDaniel Jul 27, 2021 •

edited

Loading

WenzDaniel Jul 27, 2021 •

edited

Loading