November code push ideas

[samueldmcdermott]

1. I'm satisfied that our integral calculates the same thing that pyabel would, so we can use whichever one we think is best

• 1. after making a change to the limits of integration (in short: making sure that the radius doesn't exceed R200, rather than making sure that the line of sight distance doesn't exceed R200) the shapes agree within small factors across the whole sky, which is awesome -- excellent work @kbanker !
• 2. there are a couple small visible differences which are warned about in the pyabel code as possible errors (the central pixel is a NaN and the edges have some distortion), which can be kludged, but it's not going to work perfectly out of the box -- so this is a reason to stick with our own
• 3. I haven't benchmarked the code, so I'm not sure about relative speeds; nor am I sure what our ultimate needs will be; nor am I sure what kind of gains we could get from improving our own [there are clear areas for improvement (np.trapz broadcasting, avoiding a nested for loop that includes a np.where), but I don't know what the precise impacts would be -- this is a very deep rabbit hole that we may or may not want to go down]. It's likely that pyabel is substantially faster, which might be a reason to switch, but this is not certain
• 4. it's also not certain how maintained / stable / modern pyabel is or will continue to be -- it's definitely better than preprofit (pushes within the last month) but it's a relatively small package and things could change any time

2. I think it would be better to be more transparent about specifying keyword arguments throughout, rather than unpacking **kwargs

• 1. current usage requires a user to know the names of all the variables
• 2. there are some values that are fixed in Battaglia et al., so it's nice to make that obvious
• 3. there is a "fixed cost" to changing this, and it's a little bit of a hassle, but I think it's a better approach in general

3. is there a reason that we use Pth (and P200 and P0) for the thermal pressure and then epp for the electron pressure?

• 1. I think Pe might make it a little more clear that they're related
• 2. there are some constant conversion factors that get calculated every time epp_to_y is called which we could make globals (not very important, just kind of nice)

4. in general it might be good to allow more flexible usage of either a dictionary from load_vars or the individual components

• 1. the way I've done this in the past is to have redundant keyword arguments that all default to None and then test for which has been passed (eg, cosmo = user_dict['cosmo'] if user_dict is not None else cosmo)
• 2. this is kind of related to 2.

5. as a general point, it's not totally clear what the "flow" of make_sz_cluster is: which function do you most want the user to use, generate_y_submap or simulate_submap? The names are difficult to tell apart.

• 1. at the very least, I think simulate_submap->simulate_submaps is better
• 2. these might be best off as a single function (or class) which tests to see how many clusters you're asking for (maybe defaulting to a single submap)

6. I think saving maps and outputs should not be a default

• 1. in current usage, if you accidentally make a million maps with simulate_submap they'll all get saved and you could crash your computer because they're a few MB apiece
• 2. at a minimum, we should allow the option to not save these outputs
• 3. even safer, we should do some bounds checks to override the user if they want to save an "unhealthy" amount of data

This is all suitable for discussion before we include them in the code, but for several I've already made partial implementations which I can push if we agree. For the others, we should discuss how to divide work, given the time horizon we're working with.

[samueldmcdermott]

• the integral stuff is appropriate for v0
• let's work on this to make this easiest for users
• let's use Pe
• continue working on 4
• work on 5, make this clear
• work on 6 and make this reasonably safe