Conversation
bpalmeiro
left a comment
bpalmeiro
left a comment
There was a problem hiding this comment.
The PR is fine, corrects some old issues as the detector hardcoded parameters and some other parameters that were not properly transmitted throughout the chain.
My only issue is the test, not sure about it, maybe another pair of eyes could be helpful.
| ratio_cathodes = z_cathode_new/z_cathode_demo | ||
| assert_allclose ((previous_driftv/demo_driftv).values, ratio_cathodes) | ||
|
|
||
|
|
There was a problem hiding this comment.
My problem with this test is that it relays on the fact that, for this particular set of runs, the drift velocity is the same. So, might work as a particular numerical comparison, but is it right as a general one? Am I too picky?
There was a problem hiding this comment.
Please, @mmkekic , whenever you are available, could you take a quick look here?
|
It would be interesting if the PR adds a config for a case different from NEW, maybe @neuslopez and @ausonandres can agree on one and add it! |
ausonandres
force-pushed
the
detector_name
branch
5 times, most recently
from
0fd84f0 to
f8aca44
Compare
|
Okey, as @bpalmeiro suggested, example config file for DEMO (provided by @neuslopez) was added. Besides that, another test was also written to check that map computation works properly when new parameters are not specified in config file. |
ausonandres
force-pushed
the
detector_name
branch
from
814814b to
5b3c7e2
Compare
ausonandres
force-pushed
the
detector_name
branch
from
5b3c7e2 to
3699c4b
Compare
2 participants
This PR change inputs in some
map_builder_functions.pyfunctions to allow the user to change the detector and Z limits for drift velocity computation.