PieterTack / polycap

Polycapillary X-ray raytracing
GNU General Public License v3.0
3 stars 3 forks source link

poly1_1 prep #56

Closed PieterTack closed 4 years ago

PieterTack commented 4 years ago

initial commit: fixed some ToDos, added launch vs get_trans_eff test, added function to calculate external PC intersect coordinate

tschoonj commented 4 years ago

Looks like a typo

PieterTack commented 4 years ago

Hey Tom,

Hoe kan ik op een fatsoenlijke manier die functie polycap_photon_pc_intersect een polycap_vector3 laten returnen, maar tegelijk ook een soort error value laten weergeven? Zowel -1 als NULL gaan uiteraard niet, aangezien dat geen polycap_vector3 zijn.

Had nu dus als idee om een pointer te laten returnen, maar dan krijg ik (terecht) een waarschuwing dat ik address van lokale variabele aan het returnen ben... Hoe fix ik dit? Of stel je voor dat ik toch beter een int ofzo laat returnen, en de polycap_vector3 als pointer argument in de functie plaats en dan zo de waarde aanpas? Kvroeg me af wat als meest elegante oplossing wordt beschouwd.

PieterTack commented 4 years ago

Er is blijkbaar een meson issue. Appveyor werkt normaal wel (op lvserver compiled het en passen de tests)

Ik probeer ook eens te kijken om die LGTM test ook C te laten runnen. Enkel Python is hier niet echt nuttig ;)

tschoonj commented 4 years ago

Zal dat meson issue fixen.

LGTM werkt niet omdat de build dependencies vereist waarvan zij op de hoogte moeten gebracht worden door een config file.

PieterTack commented 4 years ago

@tschoonj Kijk je even polycap.pyx na mbt return values? Het belangrijke is dat als polycap_photon_launch 0 of 1 returned dan raakten de fotonen in het optic, en wordt een bijhorende transmissieefficiencite meegegeven (bij return value 0 zal dit ongeveer 0 zijn) Bij polycap_photon_launch return 2 raakte het foton de capillaire rand, en wordt in Photon.launch() NULL returned Bij -1 en -2 wordt (respecteivelijk error en foton mist optiek) ook NULL returned, maar wordt ook een error/exception gezet die kan opgevangen worden? Om dan uit meerdere launches de Source.get_transmission_efficiencies() na te bootsen, kan dan dus adhv aantal NULL en 0,1 returns bepaald worden wat de open area is, en zou dus hetzelfde resultaat moeten bekomen worden (dit klopt alvast in C code)

Enige waar ik niet zeker van ben is of je default bij return -1 of -2 direct die error ziet, of dat je soms verkeerdelijk het zou kunnen interpreteren als een 2 return (door NULL return).

PieterTack commented 4 years ago

op mijn TODO lijst voor de nieuwe versie release staat verder nog:

tschoonj commented 4 years ago

Zie eens naar:

https://github.com/PieterTack/polycap/blob/d795873577c423339cb45e4b412ef18a2eda6650/python/polycap.pyx#L458-L470

Als de error na de call niet NULL is zal er altijd een exception ge-raised worden. Zoals ge ziet wordt er indien er geen exception geproduceerd is enkel gekeken naar rv == 0: indien waar wordt er None ge-returned. Je zal die conditie moeten veranderen als er iets speciaal moet gebeuren bij rv == 2.

PieterTack commented 4 years ago

closes issue #55

tschoonj commented 4 years ago

LGTM is fixed with #57 . See build at https://lgtm.com/logs/b435fa930a69fcc93d8ffdd1454a694a811b4bb1/lang:cpp

lgtm-com[bot] commented 4 years ago

This pull request fixes 2 alerts when merging 4b57aee060bf17018dd84188f9a14ab4fa134469 into d795873577c423339cb45e4b412ef18a2eda6650 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 4 years ago

This pull request fixes 2 alerts when merging bb50a0817d3143a336e230755990627f386ee6db into d795873577c423339cb45e4b412ef18a2eda6650 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 4 years ago

This pull request fixes 2 alerts when merging f7a812f8af2ad2bf9e6e60567c4b331cd89d58b3 into b91a43424f00cf048c2a790087e3cc04926c242c - view on LGTM.com

fixed alerts: