Adapt GUI to latest changes
Opened this issue · 13 comments
The GUI is broken since d0263ba. In order to fix it, the routines celes/src/gui/app2simulation.m
and celes/src/gui/simulation2app.m must be adapted to the new constructor and data structure of the celes_simulation class.
my attempts so far (on the fix-GUI branch) failed. The simulation runs, but I'm still getting errors from the GUI. I'm not sure if the approach I'm following for #12 can be of help here, unless we use the app to write a parameter struct to pass to the struct2celes function.
What are the errors that you get?
I've never used the GUI so far so I'm not sure I'm even launching it correctly. Am I just supposed to double-click on the .mlapp file? If so, then the GUI starts throwing this error
`Error using matlab.ui.control.internal.model.AbstractNumericComponent/set.Value (line 110)
'Value' must be a double scalar.
Error in CELES_model_wizard/UpdateIncomingPower (line 173)
app.IncidentFluxEditField.Value = initial_power_wavebundle_normal_incidence(simulation);
Error in CELES_model_wizard/startupFcn (line 275)
app.UpdateIncomingPower;
Error in CELES_model_wizard (line 1172)
runStartupFcn(app, @startupFcn)`
if I then click the run button, the simulation runs correctly but then throws another error at the end due to the fact that I have moved the app2output function inside app2simulation. When I click on these errors to see where they are, the App Designer is launched but fails with this generic error message: Error loading 'CELES_model_wizard.mlapp'.
Shit, I have the same. It seems that the app designer cannot load the GUI since d3a0eb1.
Strangely, the error message reads "Error loading 'CELES_model_wizard4.mlapp'." Do you also have that?
What does the 4 mean?
Unfortunately, I cannot remember with what Matlab version I did that change then, maybe it has to do with the version.
No, I don't see the 4. That's strange
Do you have Matlab 2016b available? It seems that there, the app designer would open the GUI.
Right now, we have the unfavourable situation that during the startup of the GUI, a simulation object is initialized (in the method UpdateIncomingPower).
That didn't use to be a problem, but since the lookups are computed already during the initialization of the simulation object, this means that the lookups are computed during the startup of the GUI. This is not ideal, as the lookups need to be recomputed after the user has set his input.
There are two options - either changge the function initial_power_wavebundle_normal_incidence such that it accepts also other input than simulation objects - or, inside the GUI we work with pseudo simulation objects that are actually structs, and only when the user hits "RUN", an actual simulation object is initialized ... other ideas?
Right now I don't have a R2016b installation.
Where is this UpdateIncomingPower method defined? I cannot find it.
Regarding the options you propose, I think using temporary structs might work, which we could then turn into a simulation instance using the struct2celes stub that I've uploaded on the load/save branch.
Sounds good! UpdateIncomingPower is a method of the GUI class. It just calls app2simulation and then initial_power_wavebundle_normal_incidence to put that value into the respective editable field of the GUI. If we had some app2struct instead, and could call initial_power_wavebundle_normal_incidence with the struct instead, we would probably resolve some issues - and definitely avoid the assembly of the lookup table during GUI startup.
To me, it seems more appropriate that initial_power_wavebundle_normal_incidence requires a simulation object rather than a structure as input. As it is now, initial_power_wavebundle_normal_incidence queries some derived properties such as k_medium and omega that are calculated at construction time, and I would prefer not to store their values nor duplicate the code that calculates them.
On the contrary, is it necessary to have an editable field showing already some value at the GUI startup? Could we just remove this field, or maybe let the user specify initialField.amplitude instead? This would be compatible with storing the input parameters in auxiliary structs.
The only thing that initial_power_wavebundle_normal_incidence does with the input variable is to look at its fields. So, as long as a struct would have the same fields, the function could handle structs as well as celes_simulation objects.
Yes, in principle we could remove the beam power field. The idea was to allow the user to either specify the beam strength through the field amplitude or to the initial power. So, if one changes the one, also the other needs to update ... this is where that callback is for.
Ah sorry now I get what you mean with derived property. Well, then we might need to find another solution (like removing the field as you suggested).