Getweight

[PieterTack]

deze build faalt. Segmentation fault in tests/capil.c, maar geen idee waarom of waar. Heb valgrind geprobeerd, maar raak er niet wijzer van: valgrind --leak-check=full --track-origins=yes --show-reachable=yes ./src/polycap ../example/dub_foc.inp Zou eigenlijk valgrind op de make check moeten doen, maar geen idee hoe.

In python stuk heb ik ook nogal raar gedaan vrees ik... krijg er heel pak warnings en errors, wss omdat ik syntax ergens fout heb nagelaten...

[tschoonj]

Ge hebt in elk geval uw branch verkeerd genomen: uw commits uit de vorige pull request zitten hier ook :-)

Doe het volgende (terwijl ge op de getweight branch zit):

git fetch
git rebase origin/master
# als er geen problemen zijn doe dan:
git push -f

In het vervolg moet ge altijd een nieuwe branch maken tov master:

git checkout master
git pull
git checkout -b new-branch-name # maakt een nieuwe branch en gebruikt hem direct

[tschoonj]

Ok thanks, zal even wat debuggen.

[PieterTack]

Ah, de weights :) Of course... silly me.

[tschoonj]

Bij mij heeft valgrind die makkelijk gevonden... bizar

[PieterTack]

Maar wat wat je command line dan voor valgrind? Want ik had bij mij de indruk dat hij het gewoon op de normale code deed, maar daar het fout in de tests zelf zat gaf hij niks merkwaardig weer.

Dus maw: hoe run ik valgrind tijdens de make check?

[tschoonj]

Zonet mijn commit gefikst voor nog wat unused variable warnings eruit te halen.

Je zal nu moeten git pull --rebase doen bij u.

Om valgrind te gebruiken moet je eerst zien dat alle test executables gebuild zijn. Dus:

make
make check

Daarna ga je in tests en doe je bijvoorbeeld:

valgrind --tool=memcheck ./capil

[PieterTack]

ah, I see :) Thanks!

[PieterTack]

Kprobeer die double **weights in polycap_photon_launch te implementere. Beetje gevecht met die pointers to pointers, maar heb nu toch een 'werkende' versie. Output transmissie efficientie is echter fout (lager dan anders). Kan dit zijn omdat ik in polycap_photon_launch zelf malloc, maar deze functie vervolgens meerdere malen wordt geroepen voor ik eigenlijk de kans heb om weights te free'en?

Kweet niet of ik, in het oog van de veranderingen die ook in polycap-source.c dienen gemaakt te worden niet meer fan ben van een normale polycap_photon_get_weights functie. Dat is dan natuurlijk wel een hoop veranderingen die we net gemaakt hebben die dan strikt genomen niet nodig waren, maar oke :P

[tschoonj]

Ben je zeker dat deze commit verantwoordelijk is voor de foute transmissie efficiëntie of is dat al eerder gebeurd?

Ik denk dat je best de unit test aanpast zodat de bekomen transmissie efficiëntie vergeleken wordt met de verwachtte, zodat we direct weten als er iets misgaat.

[PieterTack]

Hm, heb snel een testje gerund (basically snel mijn laatste aanpassingen zoveel mogelijk ongedaan gemaakt) en de efficientie blijft lager. Probleem heeft zich blijkbaar dus al eerder voorgedaan. Khad inderdaad eerder al een unit test hiervoor moeten maken. Geen idee waarom ik dat destijds over het hoofd heb gezien.

Hoe kan ik in git terugrollen naar vorige versies? Kzou dat wat willen proberen om te zien of ik daadwerkelijk nog de oude resultaten terugkrijg, of dat er iets anders vreemd aan de hand is. Immers met de laatste veranderingen (energies naar polycap_photon_launch ipv polycap_photon_new) zie ik niet in hoe dat plots invloed kan hebben op de transmissie efficienties, behalve dan als er memory issues zijn in geslopen. Kzou bijvoorbeeld enerzijds terug willen naar 28a0186 (versie waarin al die figuren van poster gemaakt zijn, toen ik in Grenoble zat nog) en dan naar ad9ba05 (versie voor ik nu met die double **weights ben beginnen spelen)

[tschoonj]

Teruggaan naar een vroegere commit doe je door git checkout checksum

Met checksum bedoel ik de string met hexadecimale karakters die elke commit uniek maakt.

Probeer eens git log en je zal de history zien, met checksums.

Je kan ook eens git bisect bekijken, een git commando dat veel gebruikt wordt in situaties als deze (maar nog nooit door mij...)

[PieterTack]

oke, thanks :) Zal dat eens proberen (die checkout, de bisect lijkt me voorlopig nog net iets te technisch, maar je weet nooit dat ik een avontuurlijke bui raak natuurlijk) En sorry dat ik het vroeg, vlak nadat ik het postte raadpleegde ik de GoogleGods en die wisten me in exact dezelfde tijd hetzelfde te vertellen. (Does this mean you are secretly a GoogleGod, or have equal powers? Interesting...)

[tschoonj]

Wel misschien heeft het feit dat ik al 10 jaar git/github gebruik er iets mee te maken 😄

Die bisect lijkt eigenlijk wel cool... ik zou daar toch eens mee moeten zien te spelen.

[PieterTack]

oke, het transmissie efficientie probleem doet zich dus voor ergens tussen 28a0186 en ad9ba05. Zal dus in de 28a0186 versie een unit test ontwerpen (we weten immers dat die sort-of overeenkomt met experimentele waarden ;) ) en dan beginnen uitdokteren wat er mis gaat tegen ad9ba05 :)

[PieterTack]

to be merged

[tschoonj]

Hmmm nee, de weights worden niet gereturned in de python bindings.

Zal dat even fiksen.

[tschoonj]

Ben geen grote van die polycap_photon_launch int return value: die kan ofwel 0 of -1 zijn, en in dat laatste geval wordt misschien de error gezet... dat is niet cool.

Volgens mij zal je hier toch (minstens, in het geval ik iets over het hoofd heb gezien), 3 verschillende return values moeten hebben.

[PieterTack]

Mja, is gekomen doordat oorspronkelijk 0 of -1 returnde als waarde, maar met toevoegen van errors heb ik die ook als -1 gezet omdat dat een logischere 'error waarde' is.

Zal het wat aanpassen dat -1 returned bij error en anders 0 of 1, waarbij 0 dan falen en 1 slagen is ofzo. Sounds better?

[tschoonj]

yep, klinkt veel beter 👍

[tschoonj]

😃

[tschoonj]

Nu deze gemerged is: misschien eens kijken naar documentatie?

Ik had daar een paar weken terug al een beetje aan gewerkt maar niets gecommit: als je tijd en zin hebt om daar nu aan te werken open ik anders wel een PR met wat ik al gedaan heb...

[PieterTack]

Yup, ging nu inderdaad documentatie doen :) Natuurlijk eerst opzoeken wat er moet gebeuren, en wss nog wat beamtime data ertussen bekijken ook, maar wou zo dicht mogelijk tegen exrs al wat documentatie hebben staan :)

[tschoonj]

ok cool. zal binnen dik uur PR openen