-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add Müller convolver #170
Add Müller convolver #170
Conversation
I have added a testcase which (at least I assume) shows that something is still wrong with the convolver. Please have a look at https://github.com/litebird/litebird_sim/pull/170/files#diff-389ac96ea9e52593dace164e986e21012662fcfb74deb8a0780e44ea01c0b3f7R75 and let me know if you have any idea what's happening (e.g. that my assumptions are wrong)! |
During the last telecon with the |
I have an idea on which I'd like to hear your opinion: As far as I know extensive simulations of the beam shapes are done with GRASP, which form the basis of the beam a_lm we use in the simulations. Are there plans to also simulate the beam+HWP system with GRASP? As far as I understand, getting realistic beams for the full system at a small selection of HWP angles (i*pi/5 for i in [0;4]) should be sufficient for an accurate reconstruction at all other angles. Beam a_lm are small, so this would not be a storage problem, but I expect the GRASP simulations might be expensive. |
Hi Martin, I checked with Cristian Franceschet, who is in charge of running GRASP simulations for LiteBIRD, and he told me that there is no way to simulate an half-wave plate in GRASP. The closest option would be a polarizing grid, which is however not the same, and Cristian is not sure that would be representative enough. |
Oh, that's a pity... I'll continue investigating alternative approaches then. |
With the previous commit I fixed the matrix for horizontal linear polarization (there is a 1/2 in the slides here) |
I believe that the test was actually working. If you print the convolved TODs they are actually equal. The order of the objects to compare in |
Yuya Nagano (@okayamayu8) and I tried to compare what is implemented in LBsim, in ducc and in Condor, an external code developed by Yuya Nagano and Yusuke Takase (@yusuke-takase). We tried to do so in 4 different cases:
For cases 1 and 3, i.e. gaussian and symmetric beam, we use the following sky and beam: For cases 2 and 4, i.e. asymmetric beam, we use the following sky and beam: For cases 1 and 2, i.e. no HWP, we use For cases 3 and 4, i.e. with HWP, we use Below the results (convolved TODs and residuals). Everything works fine a part for case 4. We are trying to understand why. Yuya tried to convolve in 2 different ways, that are matching between them but not with LBsim. I tried case 4 without the V component, i.e. |
Thank you very much for these tests! I'm glad that we have good agreement in the simple cases, and I hope that we should be able to find and fix the discrepancies in case 4. I just did a very quick comparison of case 4 between the Mueller convolver and |
Hi:) some news about case 4 above, the only one for which we had strange results. We (mostly @okayamayu8) managed to get a nice comparison! Possible conclusion: Mueller convolver is actually right and maybe there is a bug on the beamconv side. What do you think? |
Hi! I have a question about the situation assumed by this convolver. input parameters situation I convolve the two cases with the Mueller convolver and my own function (Condor). Because, by solving the Müller matrix to find the respective tod, |
Hi @okayamayu8, your setup certainly looks correct ... I don't immediately know what is going wrong there. Do you have a snippet of code that runs case 1 and case 2 with the Mueller convolver, so that I can have a closer look? |
`import ducc0 parameter settingsfct = 1.0 #some factor, I don't now epsilon = 1e-4 #desired accuracy for the interpolation; a typical value is 1e-4 alm_size = hp.Alm.getsize(lmax, mmax=mmax) preparation rundom almdef nalm(lmax, mmax): def random_alm(lmax, mmax, ncomp, rng): #beam alms prepare muelleridentity = np.identity(4) * fct make rundom pointingptg = np.empty((nptg, 3)) to see result clearly I = 0slm[0]=0 fullconv = MuellerConvolver( fullconv_identity = MuellerConvolver( sig = fullconv.signal(ptg, alpha*0) fig = plt.figure(figsize=figsize) This is the calculation program I used. |
If I think about it, this implementation is based on the assumption of HWP, which limits the coefficients of spherical harmonic expansions such as CPP*. |
I think that the implementation should produce meaningful results even for identity Mueller matrices. I'm no longer sure that this is actually a bug ... |
Sorry, in the discussion, I seem to have confused the detector angle with the boresight angle. ① I(Ω) +Q(Ω) cos(2ψ+2xi) - U(Ω) sin(2ψ+2xi) So, if we consider cases such as α=0, xi=0, then ① and ② would have the same result. The calculation I was doing represents exactly such a case, so I do not think it is a bug in convolver. |
I tried to merge the master branch into this, to get up to date. Somehow this appears to have screwed up everything - the diff is now huge! Also I get really weird behaviour with git commands: martin@marvin:~/codes/litebird_sim$ git commit -m "try to fix"
error: unexpected argument '--diff' found
tip: to pass '--diff' as a value, use '-- --diff'
Usage: ruff format [OPTIONS] [FILES]...
For more information, try '--help'.
[mueller_convolver 0a13a34] try to fix What is happening here? It looks like the commit hooks have gone crazy, but I don't know how to fix this. |
Really weird! It is probably due to the fact that since 2022 (when this PR was opened) we upgraded a lot of tools, including the code formatter (it was black, now it's ruff). Virtually every file is listed as “changed”, but the differences are in formatting, import order, etc. This is one of the scariest situations when working with Git (see this thread. What is the output of the following command?
|
Thanks a lot for having a look at this! If I try
Maybe there is a version mismatch between the |
I should mention that I see the same problem on te |
Maybe the easiest way to proceed is that I get a clean clone of the repo and re-insert my changes. This is not a big deal. What do you think, @ziotom78 ? We won't learn much from that approach, but I'm not sure that's really important. |
Wow, this was my very first idea, @mreineck , but I feared to propose it! Yeah, I believe this is the fastest way. |
No big deal, I'll get to it right away! |
OK, this is getting a bit nasty. Our documented way of installing the package from The strange |
Hi @mreineck, I usually use the instructions we have in the documentation |
Thanks a lot, @paganol ! This looks much more straightforward then the original approach. We should probably update the instructions in or root directory then :-) |
Closing this PR, superseded by #337 |
This PR is a WIP to implement proper HWP+beam convolution, following what has been discussed in issue #13. We move the discussion here so that it's easier to comment on specific parts of the code.