This repository has been archived by the owner on Jun 5, 2024. It is now read-only.
refactor #363 #376
Merged
refactor #363 #376
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JoranAngevaare
commented
May 29, 2022
|
|
||
| st = strax.Context( | ||
| storage=tempdir, | ||
| config=dict( | ||
| nchunk=2, event_rate=1, chunk_size=1, | ||
| detector='XENON1T', | ||
| fax_config=('https://raw.githubusercontent.com/XENONnT/strax_auxiliary_files' | ||
| '/36d352580b328ff057b1588b8af8c9a6ed8ae704/sim_files/fax_config_1t.json'), # noqa | ||
| '/a5b92102505d6d0bfcdb563b6117bd4040a93435/sim_files/fax_config_1t.json'), # noqa |
There was a problem hiding this comment.
important to get the latest version
| fax_config_override=dict( | ||
| url_base=("https://raw.githubusercontent.com/XENONnT/strax_auxiliary_files" | ||
| "/36d352580b328ff057b1588b8af8c9a6ed8ae704/sim_files/"), ), | ||
| "/a5b92102505d6d0bfcdb563b6117bd4040a93435/sim_files/"), | ||
| **conf_1t, |
There was a problem hiding this comment.
and update with the test in the load-resource test
|
|
||
| @skipIf(not straxen.utilix_is_configured(), 'utilix is not configured') | ||
| def test_sim_nt_advanced( | ||
| config = None |
There was a problem hiding this comment.
We should always test the default config - this wasn't done before
Comment on lines
-124
to
-129
| st.set_config({'fax_config_override': dict(s2_luminescence_model='simple', | ||
| s2_time_model="s2_time_spread around zero", | ||
| s1_lce_correction_map='XENONnT_s1_xyz_LCE_corrected_qes_MCva43fa9b_wires.json.gz', | ||
| s1_time_spline='XENONnT_s1_proponly_va43fa9b_wires_20200625.json.gz', | ||
| s1_model_type='optical_propagation+simple',)}) | ||
|
|
There was a problem hiding this comment.
add this in a separate test
Comment on lines
+139
to
+177
| @skipIf(not straxen.utilix_is_configured(), 'utilix is not configured') | ||
| def test_nt_advanced_alt_s2_model(): | ||
| config = dict( | ||
| fax_config_override=dict( | ||
| s2_luminescence_model='simple', | ||
| s2_time_model="s2_time_spread around zero", | ||
| s1_lce_correction_map='XENONnT_s1_xyz_LCE_corrected_qes_MCva43fa9b_wires.json.gz', | ||
| s1_time_spline='XENONnT_s1_proponly_va43fa9b_wires_20200625.json.gz', | ||
| s1_model_type='optical_propagation+simple', | ||
| ) | ||
| ) | ||
| test_sim_nt_advanced(config) | ||
|
|
||
|
|
||
| @skipIf(not straxen.utilix_is_configured(), 'utilix is not configured') | ||
| def test_nt_advanced_garfield(): | ||
| config = dict( | ||
| fax_config_override=dict( | ||
| s2_luminescence_model='garfield', | ||
| s2_correction_map=False, | ||
| s2_time_model="s2_time_spread around zero", | ||
| s1_lce_correction_map='XENONnT_s1_xyz_LCE_corrected_qes_MCva43fa9b_wires.json.gz', | ||
| s1_time_spline='XENONnT_s1_proponly_va43fa9b_wires_20200625.json.gz', | ||
| s1_model_type='optical_propagation+simple', | ||
| ) | ||
| ) | ||
| test_sim_nt_advanced(config) | ||
|
|
||
| @skipIf(not straxen.utilix_is_configured(), 'utilix is not configured') | ||
| def test_nt_advanced_gas_gap_garfield(): | ||
| config = dict( | ||
| fax_config_override=dict( | ||
| s2_luminescence_model='garfield_gas_gap', | ||
| s2_correction_map="XENONnT_s2_xy_map_v4_210503_mlp_3_in_1_iterated.json", | ||
| s2_luminescence_gg= "garfield_timing_map_gas_gap_sr0.npy", | ||
| garfield_gas_gap_map= "garfield_gas_gap_map_sr0.json", | ||
| se_gain_map="XENONnT_se_xy_map_v1_mlp.json", | ||
| ) | ||
| ) |
There was a problem hiding this comment.
bunch of older test configurations that we should keep supporting
| @@ -455,7 +457,7 @@ def digitizer_saturation(data, channel_mask): | |||
| @export | |||
| class RawDataOptical(RawData): | |||
|
|
|||
| def __init__(self, config, channels=[], timings=[]): | |||
| def __init__(self, config, channels=tuple(), timings=tuple()): | |||
There was a problem hiding this comment.
Having mutable default is an absolutely disastrous idea
Comment on lines
-235
to
-243
| if ('ext_eff_from_map' not in config): | ||
| cy = electron_lifetime_correction*config['electron_extraction_yield'] | ||
| else: | ||
| if config['ext_eff_from_map'] == False: | ||
| cy = electron_lifetime_correction*config['electron_extraction_yield'] | ||
| else: | ||
| # Extraction efficiency is g2(x,y)/SE_gain(x,y) | ||
| cy = config['g2_mean']*resource.s2_correction_map(xy_int)*\ | ||
| electron_lifetime_correction/resource.se_gain_map(xy_int) |
There was a problem hiding this comment.
this looked a bit convoluted
Comment on lines
-296
to
+309
| s2_luminescence_map = straxen.get_resource(files['s2_luminescence'], fmt='npy') | ||
| self.s2_luminescence = s2_luminescence_map | ||
|
|
||
| gf_file_name = files['s2_luminescence'] | ||
| if gf_file_name.endswith('npy'): | ||
| s2_luminescence_map = straxen.get_resource(gf_file_name, fmt='npy') | ||
| self.s2_luminescence = s2_luminescence_map | ||
| elif gf_file_name.endswith('npz'): | ||
| # Backwards compatibility from before #363 / #370 | ||
| s2_luminescence_map = straxen.get_resource(gf_file_name, fmt='npy_pickle')['arr_0'] | ||
| # Get directly the map for the simulated level | ||
| liquid_level_available = np.unique(s2_luminescence_map['ll']) # available levels (cm) | ||
| liquid_level = config['gate_to_anode_distance'] - config['elr_gas_gap_length'] # cm | ||
| liquid_level = min(liquid_level_available, key=lambda x: abs(x - liquid_level)) | ||
| self.s2_luminescence = s2_luminescence_map[s2_luminescence_map['ll'] == liquid_level] | ||
| else: | ||
| raise ValueError(f'{gf_file_name} is of unknown format') |
There was a problem hiding this comment.
This is why the tests kept failing
Comment on lines
+96
to
+98
| def __init__(self, config=None): | ||
| if config is None: | ||
| config = {} |
There was a problem hiding this comment.
mutable defaults strike again
|
Hi Joran, Thank you for checking these tests, I really appreciate it! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix a few problems and testing for #363