Changes to Synchrotron Radiation from issue #4921. Part 1. #4926
Conversation
as well as uses pathlib and some comment corrections
FilipO28555
changed the title
…ncation 2) Changed the number of bins to 200 for better accuracy 3) added some printouts for clarity
- Introduced `read_photon_data` function to encapsulate the logic for reading and processing photon data from OpenPMD series iteration. - Introduced `read_electron_data` function to encapsulate the logic for reading and processing electron data from OpenPMD series iteration. - Refactored the `main` function to use the new abstraction functions for reading photon and electron data. This change improves code readability and maintainability by separating concerns and making the main function less complex.
PrometheusPi
added
component: tools
There was a problem hiding this comment.
Hi, looks good. Thanks for going the extra mile to make this a little more sustainable code. In what sense are the questions answered? They are already ticked but that information has not propagated to me. So, it is likely that whatever method you used to answer them, does not help the uninvolved future reader of your code.
share/picongpu/tests/Synchrotron_plugin/lib/change_parameters_randomly.py
Outdated
Show resolved
Hide resolved
| x1 = [x for x in x1 if x >= minX and x <= maxX] | ||
| y0 = [y for x, y in zip(x0, y0) if x >= minX and x <= maxX] | ||
| y1 = [y for x, y in zip(x1, y1) if x >= minX and x <= maxX] | ||
|
|
||
| y1 = np.interp(x0, x1, y1) # interpolate y1 to the x0 values |
There was a problem hiding this comment.
Okay, so you are saying that you are fine with the default behaviour of np.interp here? I would have assumed that for histograms using left=0, right=0 would have been the most faithful extrapolation.
There was a problem hiding this comment.
You are right. I did that. I also changed the name to be more descriptive
| @@ -114,11 +134,12 @@ def main(dataPath): | |||
| analytical_integrated = np.array(analytical_integrated)[mask] | |||
|
|
|||
| x, y = subtract_functions(delta, analytical_integrated, b, a) | |||
| poorness = np.sum(y) / len(y) # average error | |||
| poorness = np.sum(y) / len(y) # average relative error | |||
There was a problem hiding this comment.
Use np.mean (potentially with axis=0) here.
There was a problem hiding this comment.
We left it because in the future we might do something like np.sum(y*np.sqrt(a)) / len(y)
There was a problem hiding this comment.
That's still a mean isn't it? np.mean(y*np.sqrt(a))
There was a problem hiding this comment.
and why is this now "relative" ?
There was a problem hiding this comment.
It was always relative.
Also changed to np.mean()
- in np.interp() added left=0 and right=0 - subtract_functions() is now relative_error_array() - SystemExit changed to ValueError
| @@ -114,11 +134,12 @@ def main(dataPath): | |||
| analytical_integrated = np.array(analytical_integrated)[mask] | |||
|
|
|||
| x, y = subtract_functions(delta, analytical_integrated, b, a) | |||
| poorness = np.sum(y) / len(y) # average error | |||
| poorness = np.sum(y) / len(y) # average relative error | |||
PrometheusPi
added
the
CI:no-compile
and reverted the ValueError to SystemExit
PrometheusPi
merged commit 077ec93
into
ComputationalRadiationPhysics:dev
in validate():
Todos from issue #4912:
pathlibinstead ofos.pathhere and herechange_parameters_randomly.pymainfunction instead of a script like codenp.asarrayhere and herevalidate.py