galsci / pysm

PySM 3: Sky emission simulations for Cosmic Microwave Background experiments
https://pysm3.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
36 stars 23 forks source link

Implementation of HD2017 dust model #37

Closed zonca closed 4 years ago

zonca commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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

zonca commented 4 years ago

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

b-thorne commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

b-thorne commented 4 years ago

@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 commented 4 years ago

@bthorne93 sure, no problem

zonca commented 4 years ago

@bthorne93 rebase was too messy, I merged!

tests pass on my machine!

b-thorne commented 4 years ago

Great! Should I merge?

zonca commented 4 years ago

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

zonca commented 4 years ago

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 !