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

Implementation of HD2017 dust model #37

Merged
merged 19 commits into from
Apr 4, 2020
Merged

Implementation of HD2017 dust model #37

merged 19 commits into from
Apr 4, 2020

Conversation

zonca
Copy link
Member

@zonca zonca commented Dec 21, 2019

No description provided.

@zonca
Copy link
Member Author

zonca commented Mar 26, 2020

@bthorne93 I'm finally back working on this, I have implemented unit tests, they don't pass!

PySM 3 does the calculation on float32 to save memory, but I don't think that is enough to explain the difference, what do you think?

@b-thorne
Copy link
Collaborator

b-thorne commented Apr 1, 2020

@zonca My first guess would be that this is just a result of the random generation of part of the spectral model using a different seed. We would need to check exactly how check_d7_353_64.fits was produced. Was this in the original pysm2 testing directory? (I can't remember if we'd implemented it at that point)

@zonca
Copy link
Member Author

zonca commented Apr 1, 2020

I created check_d7_353_64.fits
And used the same seed, is it possible the seed has different assumptions?

@zonca
Copy link
Member Author

zonca commented Apr 1, 2020

See the notebook above here about how I created that test file

freq_ref_I = "545 GHz"
freq_ref_P = "353 GHz"
rnd_uval = true
seed = 4632
Copy link
Member Author

Choose a reason for hiding this comment

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

@b-thorne
Copy link
Collaborator

b-thorne commented Apr 1, 2020

Okay, doesn't seem to be that, the code setting the seed and doing the random generating is identical in the two versions. I'll take a deeper look now.

@b-thorne
Copy link
Collaborator

b-thorne commented Apr 3, 2020

The original pysm2 random generation was always done at nside 256, however the faulty implementation is doing the random generation on templates ud_graded to the model's nside. I've got the right results by adding an nside keyword to the model's read_map function. This might not be desirable, so can figure out a way to hard code the reading at 256 if that is preferred.

@zonca zonca marked this pull request as ready for review April 3, 2020 19:06
pysm/models/dust.py Outdated Show resolved Hide resolved
@zonca
Copy link
Member Author

zonca commented Apr 3, 2020

@bthorne93 can you also rebase on master, so the unit tests run

@b-thorne
Copy link
Collaborator

b-thorne commented Apr 3, 2020

@zonca Since it's a destructive operation, and I'm not comfortable enough with my git skills, would you mind doing that? I've added the docstring above.

@zonca
Copy link
Member Author

zonca commented Apr 3, 2020

@bthorne93 sure, no problem

@zonca
Copy link
Member Author

zonca commented Apr 3, 2020

@bthorne93 rebase was too messy, I merged!

tests pass on my machine!

@b-thorne
Copy link
Collaborator

b-thorne commented Apr 3, 2020

Great! Should I merge?

@zonca
Copy link
Member Author

zonca commented Apr 4, 2020

no, I'll make sure everything passes on travis then merge

@zonca
Copy link
Member Author

zonca commented Apr 4, 2020

Not sure why github is not showing the travis builds feedback anymore.
Anyway, all tests pass:

https://travis-ci.org/github/healpy/pysm/builds/670830673

excellent, thanks @bthorne93 !

@zonca zonca merged commit 3babad9 into master Apr 4, 2020
@zonca zonca deleted the hd17 branch April 4, 2020 02:51
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.

None yet

2 participants