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

2 LofarStMan libs #1

Closed
CedricDViou opened this issue Sep 8, 2020 · 5 comments · Fixed by #3
Closed

2 LofarStMan libs #1

CedricDViou opened this issue Sep 8, 2020 · 5 comments · Fixed by #3

Comments

@CedricDViou
Copy link

Hello,

We recently encountered some issue when using LofarStMan.
We used this repository and faced some issues that could be 32/64 bits related.
Using the version from the gitlab (https://git.astron.nl/ro/lofar/-/tree/master/LCS/LofarStMan), our problems vanished (see the differences in LofarStMan.h).
Can you confirm what version is to be used, please?

Sincerely Yours,

Cedric (Nançay)

@aroffringa
Copy link
Contributor

aroffringa commented Sep 8, 2020

This should be the latest version -- the one from RO should be removed from the repo at some point. I don't see much differences between the gitlab RO version and this version in terms of headers that could explain the differences you are seeing. What exactly is going wrong with this version?

I did notice that both versions don't compile atm with the latest casacore. I've fixed that in #2 . Maybe that solves your issue?

@CedricDViou
Copy link
Author

When plotting visibilities, the data get wrong after 2GB of raw data (between minutes to hours depending on the number of visibilities).
Autocorr end-up with a non-zero imaginary part, some crosscorr get real and this goes on and on with a periodic pattern.
See plot of the same MS (Amp VS block#) but with the 2 different librairies (left: github, right: svn.astron.nl/LOFAR/tags/LOFAR-Release-4_0_17/LCS/LofarStMan (same that gitlab RO)).
left: github_lofar-astron  right: svn astron nl_LOFAR-Release-4_0_17_LCS_

The difference I see between the 2 (3 with svn repo) /include/LofarStMan/LofarStMan.h is present in some integer declarations:

  • int / int32
  • long / int64
  • casacore::uIn / int64

For example:

  • const casacore::Block<int>& ant1() const github
  • const casacore::Block<int32>& ant1() const gitlab RO
  • const casacore::Block<int32>& ant1() const svn

Maybe this is insignificant. Maybe I just didn't compile the github code correctly.
But only the code from svn or gitlab RO seems to provide us with data that makes sense.
I'm a bit puzzled that the version I "like" better is to be removed. But there's probably a good reason.
I was hoping you could help me to make it clearer.

@aroffringa
Copy link
Contributor

Thanks for the detailed explanation. Indeed you seem to be right; there are incorrect casts in this version of LofarStMan. Not sure why, but my PR #3 should fix it. Could you give a try?

@CedricDViou
Copy link
Author

Yes, I just compared 61b42c4 and master.
Master looks good now.
Thanks for the fix and the explaination!

@aroffringa
Copy link
Contributor

Great, thanks Cedric!

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

Successfully merging a pull request may close this issue.

2 participants