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

Typos, test and some documentation #178

Merged
merged 21 commits into from
Jun 22, 2022
Merged

Typos, test and some documentation #178

merged 21 commits into from
Jun 22, 2022

Conversation

paganol
Copy link
Member

@paganol paganol commented Jun 7, 2022

This PR corrects a typo in make_bin_map, improves the test test_mapping.py, and adds some documentation.
It also improves add_dipole_to_observations, now if pointings is not provided it can be retrieved from Observation.pointings

@@ -216,11 +217,15 @@ def __init__(
)

def local_intervals(self, _):
start_time = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ziotom78, I added this small piece here to handle obs.start_time passed as an astropy.time.Time object. In this case, obs.start_time is not a number and we have to call obs.start_time.cxcsec

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @sgiardie , this is absolutely correct!

@paganol
Copy link
Member Author

paganol commented Jun 17, 2022

Hi @ziotom78, we changed a bit the destriper, now it retrives the pointing from the observations (or pointings can be passed as optional argument). The previous version failed with a rotating hwp. If you like the last changes we can merge and release 0.5.0. I think @sgiardie has also an example notebook ready.

@sgiardie
Copy link
Contributor

Hi @ziotom78 and @paganol, I did some small changes in destriper/__init__.py to solve some errors due to the shape of sim.observations and pointings. I also added the notebook I have been working on. Now the binned map from make_bin_map and the one coming from MADAM seem to agree quite well, instead the destriped map is a bit weird. I plotted that in the notebook, if you can give a look at it you can tell me what is wrong and what should I change

@sgiardie
Copy link
Contributor

I reverted the changes in destriper/__init__.py, since the errors I was having depended on not passing pointings as a list of pointings corresponding to each sim.observations. I should have done

pointings=[]
for obs in sim.observations:
    pointings.append(lbs.get_pointings(obs,
                      spin2ecliptic_quats=sim.spin2ecliptic_quats,
                      detector_quats = None,
                      bore2spin_quat=instr.bore2spin_quat,
                      hwp=lbs.hwp.IdealHWP(hwp_radpsec),
                     ))

as I am doing now.
The notebook is corrected accordingly, but of course the issue in the destriped maps is still there

@sgiardie
Copy link
Contributor

Hi @ziotom78, I removed the destriper examples from the notebook. You may want to reformat it again, though. I was not able to do that

@ziotom78
Copy link
Member

Hi Serena, I checked and the code looks ok, black does not need to change it. You can merge it, thanks!

@sgiardie sgiardie merged commit 472fc3c into master Jun 22, 2022
@paganol paganol deleted the small_fixes branch June 22, 2022 15:09
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 this pull request may close these issues.

3 participants