oasys-elettra-kit / OASYS1-Wiser

The implementation of WISER into OASYS
MIT License
0 stars 1 forks source link

Slit widget is broken? #56

Closed capitanevs closed 3 years ago

capitanevs commented 3 years ago

We have won a new bug immagine

by the way, it is physically correct that "Slits" has no figure error :-) What has happened?

aljosahafner commented 3 years ago

I have no clue... This is new, I haven't changed anything

capitanevs commented 3 years ago

Yep, I suspected it. Maybe I did :)

Inviato da myMail per Android Mercoledì, 21 Luglio 2021, 10:02AM +02:00 da aljosahafner @.*** :

I have no clue... This is new, I haven't changed anything — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub , or unsubscribe .

capitanevs commented 3 years ago

The "Slits" optical element properli works in LibWiser.

The codes below (.py and .ow) represent the same layout, but error arises in .ow. The topic should be investigated.

Beamline as seen in LibWiser

====================
Beamline Name:

*[s]*           (I), dZ=0.00 m, Z=0.00 m, (I), Lambd=50 n, Waist=1 f, M2=1 
*[slits_v]*     (V), dZ=50.00 m, Z=50.00 m, (V), L=1 m, <-->
*[det_v]*       (V), dZ=100.00 m, Z=150.00 m, (V), L=50 m
====================

Beamline as seen in OASYS

immagine

test_slits_simple.RENAME_AS_PY.txt

diffraction from slits_rename_as_OWS.txt

aljosahafner commented 3 years ago

I updated to the newest version of LibWiser, but the error persists. I will investigate in these days.

aljosahafner commented 3 years ago

Bug fixed in 0.3.2. There has been code in OWOpticalElement which is the parent element also for OWSlits. It was checking for CoreOptics.FigureErrors. I added another if sentence to skip that part of the code.

Please close the issue when you confirm that the bug is indeed fixed.

capitanevs commented 3 years ago

Ok, the figure error now works properly.

Q: which kind of sentece did you use? an if, a try?

The question arise from the fact that OWOpticalElement is generic, so it should not contain something very specific like if type(...) is Optics.Slits then SKIP, but something more generic like a try or a if ... has figure error.

capitanevs commented 3 years ago

One more fix: GrazingAngleNominal as it appears in the GUI and as it appears in LibWiser does not match (2deg vs 1.57rad=90deg, which is the default value)

Perhaps such a parameter is not present inOWOpticalElement and requires extra coding? If so, I would reccomend to create an intermediate OWOpticalNumerical that can be used for a wide class of optical element, sharing the same parameters. Even if those parameters are not so many, we would avoid missteps. => to be discussed

immagine

aljosahafner commented 3 years ago

Ok, the figure error now works properly.

Q: which kind of sentece did you use? an if, a try?

The question arise from the fact that OWOpticalElement is generic, so it should not contain something very specific like if type(...) is Optics.Slits then SKIP, but something more generic like a try or a if ... has figure error.

Yes, you're right! I changed the if sentence to check whether FigureErrors exists.

aljosahafner commented 3 years ago

One more fix: GrazingAngleNominal as it appears in the GUI and as it appears in LibWiser does not match (2deg vs 1.57rad=90deg, which is the default value)

Perhaps such a parameter is not present inOWOpticalElement and requires extra coding? If so, I would reccomend to create an intermediate OWOpticalNumerical that can be used for a wide class of optical element, sharing the same parameters. Even if those parameters are not so many, we would avoid missteps. => to be discussed

immagine

Yes, my mistake. I haven't linked the AngleGrazing in Oasys. Now it works!

capitanevs commented 3 years ago

Good, that was an easy one:) Q: Some of the input paramrters (eg L and Input grazing angle) are commons to all the elements. Do ve have a strategy to ensure that they are always linked to the gui? It would be good to find one.

Inviato da myMail per Android Mercoledì, 25 Agosto 2021, 04:43PM +02:00 da aljosahafner @.*** :

One more fix : GrazingAngleNominal as it appears in the GUI and as it appears in LibWiser does not match (2deg vs 1.57rad=90deg, which is the default value) Perhaps such a parameter is not present inOWOpticalElement and requires extra coding? If so, I would reccomend to create an intermediate OWOpticalNumerical that can be used for a wide class of optical element, sharing the same parameters. Even if those parameters are not so many, we would avoid missteps. => to be discussed Yes, my mistake. I haven't linked the AngleGrazing in Oasys. Now it works! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub , or unsubscribe . Triage notifications on the go with GitHub Mobile for iOS or Android .

capitanevs commented 3 years ago

Great, I ll close the issue! tnx

Inviato da myMail per Android Mercoledì, 25 Agosto 2021, 04:35PM +02:00 da aljosahafner @.*** :

Ok, the figure error now works properly. Q: which kind of sentece did you use? an if, a try? The question arise from the fact that OWOpticalElement is generic, so it should not contain something very specific like if type(...) is Optics.Slits then SKIP, but something more generic like a try or a if ... has figure error. Yes, you're right! I changed the if sentence to check whether FigureErrors exists. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub , or unsubscribe . Triage notifications on the go with GitHub Mobile for iOS or Android .

aljosahafner commented 3 years ago

We have a strategy - bugfree coding :) . Here, the issue was that I simply forgot that the angle is also a parameter for the slit.

capitanevs commented 3 years ago

Bugfree coding is not funny :-)

I understood your point.

Ultimately you link to the gui settings here (for detector)

so one could think about making a ** dictionary containing all the standard parameters, and then pass them to Optics.WhateverElement(....). But only for L and alpha it is unpractical :-)

Il 26/08/2021 10:21, aljosahafner ha scritto:

We have a strategy - bugfree coding :) . Here, the issue was that I simply forgot that the angle is also be a parameter for the slit.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/oasys-elettra-kit/OASYS1-Wiser/issues/56#issuecomment-906198714, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADGPG3D6Z4EWO5EGSBYG4WLT6X2PZANCNFSM5AVL4IQA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.Web Bug from https://github.com/notifications/beacon/ADGPG3GG37RVWYNRZCF53I3T6X2PZA5CNFSM5AVL4IQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOGYBX5OQ.gif

-- Michele Manfredda, PhD Scientist Photon Transport Group Elettra - Sincrotrone Trieste (IT)

"The release date is just one day, but the record is forever." Bruce Springsteen

capitanevs commented 3 years ago

Still not working: now it is linked but the stored value is in degree, while it must be in radians.

I revise my previous statement: Bugfree coding is nice, but it took a lot of time to debug this single point.

I would advice refactoring the code in order to "seal" the behavior of the most common parameters.

def f (L = 1, AngleGrazingNominal=2, c=3):

    return L*1000

UniversalParameters = {'L' : 10, 'AngleGrazingNominal' :10}

#%%

f(c=3, **UniversalParameters)
capitanevs commented 3 years ago

Bug fixed.

Let's consider for the future the strategy of "SharedParameters/UniversalPArameters", etc.