Samreay / Marz

The Marz redshifting program is intended for use in cosmology surveys, specifically for the OzDES team.
17 stars 18 forks source link

Some patches #132

Closed saimn closed 8 years ago

saimn commented 8 years ago

Hi,

First, thanks a lot for open sourcing Marz, it's awesome ! :smile:

I'm giving a try to use it with MUSE data, which needs some adaptations (different templates, wavelength range, lines, ...), but is really promising. I can customize the code for our application, but I would be happy to contribute as much as possible the patches that are generic enough.

So here are several commits (I can split in multiple pull request if you prefer, or remove some commits):

Then, there are quite a number of hardcoded values that must be changed when working with a different dataset (for example for callouts: defaultMin, defaultMax, ...). It would be easier to have these values in the config files, maybe with a more structured (JSON) format ?

Samreay commented 8 years ago

Hi Simon,

Great to see these patches, I had no idea other groups might have been looking at Marz and always good to see work actually being used. In regards to the patches, I'm in full support and would be happy to merge in as soon as I can clarify the inverse variance issue (commented in the diff section).

Regarding hardcoded variables, there are quite a few, I definitely agree - when I started coding this I didn't think it would get too much use and so adaptability wasn't my highest priority (alas), and on that note please forgive my lack of documentation in the code itself.

For defaultMin and defaultMax, this shouldn't be an issue for another survey, as that default range is only used when no data is loaded in. As soon as a file is dropped in, the extents will resize. Re the third value in the callouts array, that is the relative importance of viewing that cutout (used in a later function). I wanted to instead analyse the strength of features inside the cutout and use that to rank them (to only display the most informative callouts), but I was unable to get a rigorous algorithm working in the time I allowed myself to look at it.

Thanks for getting in contact!

saimn commented 8 years ago

Thanks for your answer, I have adressed the inverse variance point. Concerning hardcoded variables and undocumented features, I will open other issues / PR later ;-). Good point for defaultMin and defaultMax, I saw later that the min/max of the data is used so it is not an issue.

Samreay commented 8 years ago

Yup, looks good then. I'll commit some comments for documentation re your template issues in just a tick.