Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt GUI to latest changes #9

Open
AmosEgel opened this issue Dec 15, 2017 · 13 comments
Open

Adapt GUI to latest changes #9

AmosEgel opened this issue Dec 15, 2017 · 13 comments
Assignees

Comments

@AmosEgel
Copy link
Contributor

AmosEgel commented Dec 15, 2017

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.

@AmosEgel
Copy link
Contributor Author

It seems that this issue depends on #12

@lpattelli
Copy link
Contributor

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.

@AmosEgel
Copy link
Contributor Author

What are the errors that you get?

@lpattelli
Copy link
Contributor

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'.

@AmosEgel
Copy link
Contributor Author

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.

@lpattelli
Copy link
Contributor

No, I don't see the 4. That's strange

@AmosEgel
Copy link
Contributor Author

Do you have Matlab 2016b available? It seems that there, the app designer would open the GUI.

@AmosEgel
Copy link
Contributor Author

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?

@lpattelli
Copy link
Contributor

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.

@AmosEgel
Copy link
Contributor Author

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.

@lpattelli
Copy link
Contributor

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.

@AmosEgel
Copy link
Contributor Author

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.

@AmosEgel
Copy link
Contributor Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants