minor bug in hwp_sys.set_parameters()

[mj-gomes]

When using the hwp_sys.py code using Mueller as the input matrix (in set_parameters) I think I found one small bug, because I get this error:

File "/home/mgomes/litebird/lbs_env/lib/python3.9/site-packages/litebird_sim/hwp_sys/hwp_sys.py", line 1071, in set_parameters
self.betas = np.deg2rad(self.betas)
AttributeError: 'HwpSys' object has no attribute 'betas'

This is my set_parameters call:

hwp_sys.set_parameters(
    mueller_or_jones="mueller",
    Mbsparams=Mbsparams,
    Channel = lbs.FreqChannelInfo(bandcenter_ghz=140)
)

This is a code snippet of the region of hwp_sys.py code where the error comes from :

if mueller_or_jones == "jones":
    default_attrs = {
        "h1s": 0.0,
        "h2s": 0.0,
        "betas": 0.0,
        "z1s": 0.0,
        "z2s": 0.0,
    }

for attr, default_value in default_attrs.items():
    if not hasattr(self, attr):
        setattr(self, attr, default_value)
    self.betas = np.deg2rad(self.betas)
else:  # mueller_or_jones == "mueller":
    default_attrs = {
        "mIIs": 0.0,
        "mQIs": 0.0,
        "mUIs": 0.0,
        "mIQs": 0.0,
        "mIUs": 0.0,
        "mQQs": 0.0,
        "mUUs": 0.0,
        "mUQs": 0.0,
        "mQUs": 0.0,
    }

for attr, default_value in default_attrs.items():
    if not hasattr(self, attr):
        setattr(self, attr, default_value)

I would say that the for block should be inside the if mueller_or_jones == "jones" block, and the last for block should be inside the else block (for the mueller case), because we don't have the beta parameter when using Mueller Matrix as input.

Also, I realized that, in this hwp.sys code, the jones matrix elements that are attributed by default when none are given correspond to the ideal case, while for the Mueller case they don't (they are all zero, instead of having the 1's in the diagonal). Is this intentional or is it something to change?

--

EDIT : I was going to create another issue, but since this error also comes from hwp_sys.set_parameters(), I add it in here. There might be another bug when giving a single frequency to the Channel parameter as in the example at the top.

File "/home/mgomes/litebird/lbs_env/lib/python3.9/site-packages/litebird_sim/hwp_sys/hwp_sys.py", line 984, in set_parameters
self.maps = mbs.run_all()[0][Channel.channel]
KeyError: '140.0 GHz'

If, in line 984 of hwp.sys, I change [Channel.channel] to ['140.0_GHz'] it works, so the problem should be either in the definition of the dictionnary keys, or in the string coming from lbs.FreqChannelInfo(bandcenter_ghz=140), because the dictionnary does not accept '140.0 GHz' without the underscore as a key, however it is the one coming from the frequency channel definition. Or maybe I should not define the frequency channel like I did?

[sgiardie]

Hi Miguel,

I would say that the for block should be inside the if mueller_or_jones == "jones" block, and the last for block should be inside the else block (for the mueller case), because we don't have the beta parameter when using Mueller Matrix as input.

Also, I realized that, in this hwp.sys code, the jones matrix elements that are attributed by default when none are given correspond to the ideal case, while for the Mueller case they don't (they are all zero, instead of having the 1's in the diagonal). Is this intentional or is it something to change?

Yes I would say you are right on both things! We should have that for block under the "jones" case, and I guess also
for attr, default_value in default_attrs.items():
if not hasattr(self, attr):
setattr(self, attr, default_value)
at line 1087 should be under the "else" ("mueller" case)

and you are correct about the default mueller values, we should have
default_attrs = { "mIIs": 1.0, "mQIs": 0.0, "mUIs": 0.0, "mIQs": 0.0, "mIUs": 0.0, "mQQs": 1.0, "mUUs": 1.0, "mUQs": 0.0, "mQUs": 0.0, }

[sgiardie]

About your other error, maybe @paganol can help
how I usually do is using
lbs.FreqChannelInfo.from_imo( url=f"/releases/{imo_version}/satellite/{instrument}/{channel}/channel_info", imo=imo, )
using the information from the IMo. You get an example in this notebook

not sure what is the issue with lbs.FreqChannelInfo(bandcenter_ghz=140)

[mj-gomes]

About your other error, maybe @paganol can help how I usually do is using lbs.FreqChannelInfo.from_imo( url=f"/releases/{imo_version}/satellite/{instrument}/{channel}/channel_info", imo=imo, ) using the information from the IMo. You get an example in this notebook

not sure what is the issue with lbs.FreqChannelInfo(bandcenter_ghz=140)

For me a simple solution would be to simply change the initialization of the attribute channel in the class FreqChannelInfo in 281 from detectors.py.

if self.channel is None:
            self.channel = f"{self.bandcenter_ghz:.1f} GHz"

to

if self.channel is None:
            self.channel = f"{self.bandcenter_ghz:.1f}_GHz"

inserting the underscore.

I tried it and I no longer get the error.

[ziotom78]

Hi @mj-gomes , thanks for having spotted these errors and for proposing a solution!

Once this is fixed, it would be good to add some tests to ensure that the error won’t appear again in the future.

Should we release a patch release (0.13.1) once a PR is merged?

[mj-gomes]

Hello @ziotom78, I will write some tests to ensure the solutions presented work, I'll get back to you when they are ready.