search

mzechmeister / serval

calculate radial velocities from stellar spectra
MIT License
36 stars 9 forks source link

tplvsini: operands could not be broadcast together with shapes (3694,) (3616,) #63

Closed mzechmeister closed 4 months ago

mzechmeister commented 4 months ago

An issue come up with the tplvsini option, yielding

operands could not be broadcast together with shapes (3694,) (3616,)

Gaps are not handled as commented in https://github.com/mzechmeister/serval/blob/ae633871c845c266710f8bf2a947355e722006cd/src/serval.py#L190-L191 I thought it was introduced in c2f47b2, since previous 8575a417276540007c0d30b37eef566986921db2 still used template.fits.

Maybe the bug was introduced in https://github.com/mzechmeister/serval/commit/a348b4ca77e57b0e9e626f8c9fb147f080cc2418#diff-862a92f4d804371ff1ad27b80246db266b84e1b1807267a7bbf74f47abc15783L1796. which stored QMAP to template.fits.

mzechmeister commented 4 months ago

It worked with 2bffc51 (commit 209, 2022-01-24) and *.tpl.fits The next commit a348b4c (commit 210, 2022-01-26) introduced https://github.com/mzechmeister/serval/blob/a348b4ca77e57b0e9e626f8c9fb147f080cc2418/src/serval.py#L171-L172 which makes the first condition in https://github.com/mzechmeister/serval/blob/a348b4ca77e57b0e9e626f8c9fb147f080cc2418/src/serval.py#L188 always true.

The issue is not present for -vsiniauto, which calls https://github.com/mzechmeister/serval/blob/ae633871c845c266710f8bf2a947355e722006cd/src/serval.py#L794 and does not mask anymore.

mzechmeister commented 4 months ago

This is not the end. -vsiniauto has issues, too.

ValueError: could not broadcast input array from shape (14797) into shape (3699)

Moreover, the lookt option plots the spline coadded template, not the vsini broadened template. This is also a bit confusing. image serval test <(head -5 autolis/J18356+329.lis) -inst CARM_VIS -targ LSRJ1835+3259 -safemode 2 -oset 43:52 -snmin 8 -tpl J02530+168 -lookt -vsiniauto -lookt -pdb -lookvsini

Actually, the lookvsini should be used. While is the optimal vsini is shown for each, in the end the same vsini should be used for all orders. (Is this case?) There seems also some issues with the masking image

(For ofacauto, different values in each order may be acceptable or not (e.g. when ofac mimics vsini).)

mzechmeister commented 4 months ago

Crosscheck that the masking is better now: image Note that the vsini dropped from 47 to 40 for this case.