LWFA: GUI .cfg & Additional Parameters #2336

ax3l · 2017-10-26T14:07:27Z

Add to python input `Parameter` class

dtype: data type of the PIConGPU value
label: UI label instead of name
formatter: UI value label instead of name

New cfg for GUI: `0004gpus_gui.cfg`

make more easily modifiable with tbg -o:
- steps
- grid size
- devices
expose TBG_steps properly as integers.
all plugins dump in same interval: TBG_plugin_period
add histogram, second cut in PNG and phase space

codingS3b · 2017-10-27T10:24:29Z

lib/python/picongpu/input/parameters.py

+        """
+
+        members = super(UiParameter, self).as_dict()
+        del members["formatter"]


Seems like we need to exclude the "dtype" member as well since it can't be serialzed either

I just found out that we could store the the dtype as a string representation using
members["dtype"] = members["dtype"].__name__. Maybe it would be cleaner to do this for later restoring the parameter objects from the json representation correctly.

Similarly it seems also possible to store the formatter lambda as a string representation (see https://stackoverflow.com/a/30984012)

ok, but careful that a lambda does not mean it is just one source line ;)

shall we add the string representation of both in a follow-up? do we need it now?
otherwise feel free to approve the PR :)

I think for the moment we can leave them out, but maybe the class of each parameter object should be stored as a string representation.
This is because, as of right now, we still have issues in our ui-code when reading back the parameter variables from json: there, we implicitely have to cast to the base class "Parameter" (since we don't have any other information) losing all info concerning the dtype and formatter which the program then tries to access, of course crashing.
So I still need to figure out a proper way to restore an "exact" copy of the parameter object on our side of the code. But the dtype and the formatter are handled with defaults when not given, so that should be not a problem.

Hold on, sorry...maybe our bug is somewhere else and we don't need the class representation. Don't change it just yet, I will get back to you :)

I found the critical section: calling del members["formatter"] seems to remove the entry not only from the members dictionary, but since it is a reference also from the true object. This should of course be avoided!

codingS3b · 2017-10-27T10:34:23Z

lib/python/picongpu/input/parameters.py

@@ -208,7 +241,8 @@ class LogScaledParameter(Parameter):
    """

    def __init__(self, name, ptype, unit, default, slider_min, slider_max,


LogScaledParameter should actually inherit from UiParameter. I commited that wrong and it needs to be fixed.

codingS3b · 2017-10-27T10:51:35Z

share/picongpu/examples/LaserWakefield/lib/python/picongpu/params.py

        slider_step=1, scale_factor=1.e-15),
+
+    LinearScaledParameter(
+        name="TBG_steps", ptype="run", unit="s",


It would probably be less confusing to use the units the user is accustomed to like pico-seconds, nano-meters etc as units. The formatted internal picongpu value of each parameter is, as discussed yesterday, indeed not relevant to the user. Sorry for switching back and forth about this issue.

codingS3b · 2017-10-27T11:21:06Z

lib/python/picongpu/input/parameters.py

-        self.value = self.base ** x
-        return self.value
+        self.value = self.dtype(self.base ** x)
+        return self.formatter(self.value)

    def get_value(self):


Before calling log10(self.value) we need to cast the value back to float, because in case we use self.dtype = int, I got the error AttributeError: 'int' object has no attribute 'log10'

codingS3b · 2017-10-27T11:35:36Z

share/picongpu/examples/LaserWakefield/lib/python/picongpu/params.py

        slider_step=1, scale_factor=1.e-15),
+
+    LinearScaledParameter(
+        name="TBG_steps", ptype="run", unit="s",


For the Wave_Length_SI parameter the default should be a float or the dtype should be specified as float, since right now the dtype is taken as int causing much trouble in the computations that follow.

Same goes for the Base_Density_SI

- make more easily modifiable with `tbg -o`: - steps - grid size - devices - all plugins dump in same interval

Add to python input parameter class: - dtype: data type of the PIConGPU value - label: UI label instead of name - formatter: UI value label instead of name

Expose TBG_steps properly as integers.

ax3l · 2017-10-27T12:15:50Z

@codingS3b thanks for the review, I addressed all inline comments.
can you pls take a look again?

PrometheusPi

very good code - just some remarks and an evil magic number

PrometheusPi · 2017-10-27T14:09:08Z

lib/python/picongpu/input/parameters.py


        """
-        Parameter.__init__(self, name, ptype, unit, default, value)
+        Parameter.__init__(self, name, ptype, unit, default, value, dtype)


QUESTION:
dtypeis (as far as I can tell) not used below, why is is then used as input parameter?
Add a todo if you want to add functionality using dtype in the future.

dtype is used to init a member of the parent class.

it is then used in all set_value

Sorry for the confusion - my fault - it is used right there

PrometheusPi · 2017-10-27T14:14:53Z

share/picongpu/examples/LaserWakefield/lib/python/picongpu/params.py

@@ -18,10 +18,13 @@

 from picongpu.input.parameters import LogScaledParameter, LinearScaledParameter

+pico = 1.e-12
+dt_r = 1. / 1.39e-16 * pico


REQUIRED:
Please explain this magic number (1.39e-16) in a comment or give it a name that makes it clear (as e.g. pico).

this is the inverse (historically suffixed _r) of dt (in inverse ps).

We might be able to just add is as a Parameter (but not UiParameter) in the list below to make it more accessible.

Currently, we do not have a clean interface for it :(

PrometheusPi · 2017-10-27T14:17:33Z

share/picongpu/examples/LaserWakefield/etc/picongpu/0004gpus_gui.cfg

+TBG_e_PSypy="--e_phaseSpace.period !TBG_plugin_period --e_phaseSpace.space y --e_phaseSpace.momentum py --e_phaseSpace.min  0.0 --e_phaseSpace.max 1.0"
+TBG_e_PSzpz="--e_phaseSpace.period !TBG_plugin_period --e_phaseSpace.space z --e_phaseSpace.momentum pz --e_phaseSpace.min -2.0 --e_phaseSpace.max 2.0"
+
+TBG_plugins="!TBG_pngYX                    \


QUESTION:
Why is this so much different to the other *.cfg files that it needs to be a separate file?
Couldn't we just use a combined *.cfgfile to avoid duplicated code/pile up.

We want to control steps directly from the outside, but our tools do not understand -s number but only tbg variables such as !TBG_steps, controlled with tbg -o.

We will also add probably more changes here, so it is best to make it a fully separate setup.

Okay - that's a good idea 👍

ax3l · 2017-11-01T15:34:41Z

@PrometheusPi @codingS3b if you are ok with the intermediate state of this feature we can already merge it so @codingS3b can work on with the dev

PrometheusPi

accept for now - fixes will be added later

106a4975f4 fix getFunctionAttributes for the SYCL backend f36e1156af update CUDA version in CI 3f8456973e use inline for CUDA/HIP code when rdc is on, otherwise use static 8b9cc3c557 fix gh-pages jobA 89d5ce671c Ignore VI temporary files 4b7bd17493 Fix the device used by KernelExecutionFixture (ComputationalRadiationPhysics#2344) 2c386dc5e9 Make alpaka follow CMAKE_CUDA_RUNTIME_LIBRARY 2d652dd233 Add thread count to CPU blocks accelerators (ComputationalRadiationPhysics#2338) dbc5ebe1e9 Fix complex math pow test (ComputationalRadiationPhysics#2336) 4995c5b22a Fix isValidWorkDivKernel to use the correct device f571ce9197 Remove unnecessary include a26cdbcd41 Re-enable the KernelNoTemplateGpu test a9217fb780 Link libcudart even when libcurand is not used 9c8614143b Suppress GCC warning about casting a function to void* ba169cdc52 Rewrite the getValidWorkDivForKernel tests 948eb757d4 Fix getValidWorkDivForKernel tests for the SYCL CPU backend f6f94f13b5 Fix getValidWorkDivForKernel tests for the CUDA backend f612f971a0 Reduce code duplications in matrixMulWithMdSpan (ComputationalRadiationPhysics#2326) d1cc2e01c1 Add a matrix multiplication example using mdspan 536a183cce Add missing whitespace in enqueue log messages 81d4410eec Reduce code duplication in CUDA/HIP kernel launch 6fdec14904 add remove-restrict 5323600508 CI: improve script utils 01d123e605 fix missing C++20 STL for ICPX in the CI d254bcd6a3 ctest: display only output of tests, which failed c9b8c941af change documentation b9ed742913 remove getValidWorkDiv itself 048ef8afca use getValidWorkDivForKernel in kernelfixture of tests 38805498f0 fix random strategies 4f175420f2 remove getValidWorkDiv first 7f08120428 CI_FILTER: ^linux_nvcc11.* 789344f019 ALPAKA_FN_HOST is not a type 4efdb9dc63 fix explicit instantiation issue fe4106f88a CI_FILTER: ^linux_nvcc11.*gcc9 e6b4881b70 CI_FILTER: ^linux_nvcc11.*gcc9 e3e760ed9e make conv2dmdspan use kernelbundle 62efffe605 Add getValidWorkDivForKernel function and KernelBundle with tests 690da679bd Let the SYCL queue implement `ConceptCurrentThreadWaitFor`, `ConceptGetDev` and `ConceptQueue` (ComputationalRadiationPhysics#2314) 995c57b54b set alpaka_CXX_STANDARD in the job generator 6ad09baa38 remove nvcc11.0 and nvcc11.1 support (ComputationalRadiationPhysics#2310) 0775f7c066 clang-format and fix typo 18eeeb7b49 move complex declaration to internal namespace 3468d2f8ac add trait IsKernelTriviallyCopyable 3015eae06b update CI container to version 3.2 56c0e416bc Update Catch2 to v3.5.2 (ComputationalRadiationPhysics#2300) git-subtree-dir: thirdParty/alpaka git-subtree-split: 106a4975f48dc38cc34f6a2494a3d16774282951

ab3092357b Use shared CUDA libraries by default 106a4975f4 fix getFunctionAttributes for the SYCL backend f36e1156af update CUDA version in CI 3f8456973e use inline for CUDA/HIP code when rdc is on, otherwise use static 8b9cc3c557 fix gh-pages jobA 89d5ce671c Ignore VI temporary files 4b7bd17493 Fix the device used by KernelExecutionFixture (ComputationalRadiationPhysics#2344) 2c386dc5e9 Make alpaka follow CMAKE_CUDA_RUNTIME_LIBRARY 2d652dd233 Add thread count to CPU blocks accelerators (ComputationalRadiationPhysics#2338) dbc5ebe1e9 Fix complex math pow test (ComputationalRadiationPhysics#2336) 4995c5b22a Fix isValidWorkDivKernel to use the correct device f571ce9197 Remove unnecessary include a26cdbcd41 Re-enable the KernelNoTemplateGpu test a9217fb780 Link libcudart even when libcurand is not used 9c8614143b Suppress GCC warning about casting a function to void* ba169cdc52 Rewrite the getValidWorkDivForKernel tests 948eb757d4 Fix getValidWorkDivForKernel tests for the SYCL CPU backend f6f94f13b5 Fix getValidWorkDivForKernel tests for the CUDA backend f612f971a0 Reduce code duplications in matrixMulWithMdSpan (ComputationalRadiationPhysics#2326) d1cc2e01c1 Add a matrix multiplication example using mdspan 536a183cce Add missing whitespace in enqueue log messages 81d4410eec Reduce code duplication in CUDA/HIP kernel launch 6fdec14904 add remove-restrict 5323600508 CI: improve script utils 01d123e605 fix missing C++20 STL for ICPX in the CI d254bcd6a3 ctest: display only output of tests, which failed c9b8c941af change documentation b9ed742913 remove getValidWorkDiv itself 048ef8afca use getValidWorkDivForKernel in kernelfixture of tests 38805498f0 fix random strategies 4f175420f2 remove getValidWorkDiv first 7f08120428 CI_FILTER: ^linux_nvcc11.* 789344f019 ALPAKA_FN_HOST is not a type 4efdb9dc63 fix explicit instantiation issue fe4106f88a CI_FILTER: ^linux_nvcc11.*gcc9 e6b4881b70 CI_FILTER: ^linux_nvcc11.*gcc9 e3e760ed9e make conv2dmdspan use kernelbundle 62efffe605 Add getValidWorkDivForKernel function and KernelBundle with tests 690da679bd Let the SYCL queue implement `ConceptCurrentThreadWaitFor`, `ConceptGetDev` and `ConceptQueue` (ComputationalRadiationPhysics#2314) 995c57b54b set alpaka_CXX_STANDARD in the job generator 6ad09baa38 remove nvcc11.0 and nvcc11.1 support (ComputationalRadiationPhysics#2310) 0775f7c066 clang-format and fix typo 18eeeb7b49 move complex declaration to internal namespace 3468d2f8ac add trait IsKernelTriviallyCopyable 3015eae06b update CI container to version 3.2 56c0e416bc Update Catch2 to v3.5.2 (ComputationalRadiationPhysics#2300) git-subtree-dir: thirdParty/alpaka git-subtree-split: ab3092357bb0b917b4cc396ce49e47f7ac1924e1

ax3l added the component: tools scripts, python libs and CMake label Oct 26, 2017

ax3l added this to the 0.4.0 / 1.0.0: Next Stable milestone Oct 26, 2017

ax3l requested a review from codingS3b October 26, 2017 14:07

ax3l force-pushed the topic-lwfaGUIcfg branch 2 times, most recently from 70e752f to 437398f Compare October 26, 2017 14:34

codingS3b suggested changes Oct 27, 2017

View reviewed changes

codingS3b reviewed Oct 27, 2017

View reviewed changes

ax3l added 3 commits October 27, 2017 14:15

LWFA: 4 Device GUI Example

50a5cf2

- make more easily modifiable with `tbg -o`: - steps - grid size - devices - all plugins dump in same interval

Python: Input dtype, label, formatter

010ed55

Add to python input parameter class: - dtype: data type of the PIConGPU value - label: UI label instead of name - formatter: UI value label instead of name

LWFA: Python Steps as Int

6ccbf5f

Expose TBG_steps properly as integers.

ax3l force-pushed the topic-lwfaGUIcfg branch from 437398f to 6ccbf5f Compare October 27, 2017 12:15

ax3l assigned PrometheusPi Oct 27, 2017

ax3l requested a review from PrometheusPi October 27, 2017 14:03

ax3l added the component: examples PIConGPU or PMacc examples label Oct 27, 2017

PrometheusPi requested changes Oct 27, 2017

View reviewed changes

Python parameters: fix as_dict() methods for correct serialization

886addf

PrometheusPi approved these changes Nov 1, 2017

View reviewed changes

PrometheusPi merged commit b6925ca into ComputationalRadiationPhysics:dev Nov 1, 2017

ax3l deleted the topic-lwfaGUIcfg branch November 1, 2017 16:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LWFA: GUI .cfg & Additional Parameters #2336

LWFA: GUI .cfg & Additional Parameters #2336

ax3l commented Oct 26, 2017 •

edited

Loading

codingS3b Oct 27, 2017

codingS3b Oct 27, 2017 •

edited

Loading

ax3l Oct 27, 2017

ax3l Oct 27, 2017

codingS3b Nov 1, 2017

codingS3b Nov 1, 2017

codingS3b Nov 1, 2017 •

edited

Loading

codingS3b Oct 27, 2017

codingS3b Oct 27, 2017

codingS3b Oct 27, 2017

codingS3b Oct 27, 2017 •

edited

Loading

ax3l commented Oct 27, 2017

PrometheusPi left a comment

PrometheusPi Oct 27, 2017

ax3l Oct 27, 2017

PrometheusPi Oct 27, 2017

PrometheusPi Oct 27, 2017

ax3l Oct 27, 2017 •

edited

Loading

PrometheusPi Oct 27, 2017

ax3l Oct 27, 2017

PrometheusPi Oct 27, 2017

ax3l commented Nov 1, 2017

PrometheusPi left a comment

		@@ -208,7 +241,8 @@ class LogScaledParameter(Parameter):
		"""

		def __init__(self, name, ptype, unit, default, slider_min, slider_max,

LWFA: GUI .cfg & Additional Parameters #2336

LWFA: GUI .cfg & Additional Parameters #2336

Conversation

ax3l commented Oct 26, 2017 • edited Loading

Add to python input Parameter class

New cfg for GUI: 0004gpus_gui.cfg

Choose a reason for hiding this comment

codingS3b Oct 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codingS3b Nov 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codingS3b Oct 27, 2017 • edited Loading

Choose a reason for hiding this comment

ax3l commented Oct 27, 2017

PrometheusPi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ax3l Oct 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ax3l commented Nov 1, 2017

PrometheusPi left a comment

Choose a reason for hiding this comment

ax3l commented Oct 26, 2017 •

edited

Loading

Add to python input `Parameter` class

New cfg for GUI: `0004gpus_gui.cfg`

codingS3b Oct 27, 2017 •

edited

Loading

codingS3b Nov 1, 2017 •

edited

Loading

codingS3b Oct 27, 2017 •

edited

Loading

ax3l Oct 27, 2017 •

edited

Loading