LSST-nonproject / sims_maf_contrib

Contributed code for MAF (sims_maf)
18 stars 46 forks source link

Can one metric influence a FOR loop inside another metric? #26

Closed willclarkson closed 7 years ago

willclarkson commented 8 years ago

Hi @yoachim, @rhiannonlynne, all - is there a straightforward way for output from one Metric to be passed into a FOR loop in another Metric?

I'm thinking in particular of the case where measurement uncertainty depends on magnitude in a somewhat involved manner (e.g. crowding uncertainty for a particular location), but those magnitudes are generated within a loop inside a Metric (e.g. Monte Carlo iterations within a Metric that simulates lightcurves). What is the best way to pass information from another metric into the FOR loop within the Metric?

I couldn't see immediately how to do this using two metrics (e.g. taking output from the CrowdingMetric into the periodicStarMetric by @yoachim), so tried a different approach, with the crowding estimate put into a separate module that the lightcurve Metric can call within each iteration of the FOR loop. That seems to work reasonably well (I just submitted a pull request with the new module), but if there is a more Metric-centric way to accomplish this, it would be great to know.

For interest: @knutago

Thanks!!

Will

rhiannonlynne commented 8 years ago

Hi Will,

I think in this case what you've done is quite reasonable -- Knut's metric which calculated the crowding limit is something which would be very useful beyond his own use-case (for his own use, being a metric makes a lot of sense .. for use by other people, transforming it into something else also makes a lot of sense ... I could have seen turning it into a map, actually, similar to the dust map which returns EBV at each slicePoint, actually, then being something you could invoke or not for any set of metrics when you instantiate them).

However, it actually is pretty easy to pass the output of one metric to another -- you can just call it from within your metric.

Each metric has an __init__ and a run method, and the run method API is on purpose very generic. So what you could have done was:

The thing to be careful of is our framework is not quite smart enough to automatically detect all of the columns needed for this "hidden" metric, so you'd have to add them to the columns requested by your metric.

Does that make sense? I can try to make an example if that would be helpful.

willclarkson commented 8 years ago

Thanks Lynne! If I understand you, the pseudocode for what you suggest would look something like the lines below. I will try this out when I next get a chance.

Just one question: does the new crowding error metric CrowdingMagUncertMetric() that Peter implemented in sims_maf_contrib, accept magnitudes as a vector? Or does it accept only a scalar fiducial magnitude for now?

Best -- Will

initialise Crowding Error metric with dummy array of magnitudes

crowdError = CrowdingMetrics.crowdingErrorMetric(dataslice, slicePoint) dummyMags = [some vector of magnitudes] crowdError.fiducialMags = dummyMags

for i in range(nTrials):

       # generate the i'th trial lightcurve, using it to update
       # the crowding error computed by the crowding metric
       crowdError.fiducialMags = self.makeFakeLightcurve(params)
       crowdError.run(dataslice, slicePoint))

       # retrieve the updated crowding error and apply it to
       # the lightcurve in this trial
       crowdSigma = np.copy(crowdError.oneSigmaValues)
       thisLightcurve = applyCrowdToCurve(thisTrialLightcurve,

crowdSigma)

Best

Will

Dr. Will Clarkson Assistant Professor of Physics and Astronomy University of Michigan-Dearborn Students: please put your course code in the title of emails to me (ASTR330 or PHYS 150 lab or discussion).

On Mon, Jan 4, 2016 at 11:00 AM, Lynne Jones notifications@github.com wrote:

Hi Will,

I think in this case what you've done is quite reasonable -- Knut's metric which calculated the crowding limit is something which would be very useful beyond his own use-case (for his own use, being a metric makes a lot of sense .. for use by other people, transforming it into something else also makes a lot of sense ... I could have seen turning it into a map, actually, similar to the dust map which returns EBV at each slicePoint, actually, then being something you could invoke or not for any set of metrics when you instantiate them).

However, it actually is pretty easy to pass the output of one metric to another -- you can just call it from within your metric.

Each metric has an init and a run method, and the run method API is on purpose very generic. So what you could have done was: in the init of your metric, instantiate one of Knut's metric and set it up appropriately with the fiducial magnitude, etc, saving the metric object as a class attribute. (e.g., as mymetric.crowdingLimitMetric) in the run method of your metric, call Knut's metric's run method. (e.g. crowdinglimit = mymetric.crowdingLimitMetric.run(dataslice, slicePoint))

The thing to be careful of is our framework is not quite smart enough to automatically detect all of the columns needed for this "hidden" metric, so you'd have to add them to the columns requested by your metric.

Does that make sense? I can try to make an example if that would be helpful.

— Reply to this email directly or view it on GitHub https://github.com/LSST-nonproject/sims_maf_contrib/issues/26#issuecomment-168715672 .

rhiannonlynne commented 8 years ago

I'll tag @yoachim here as he may be able to answer better. It looks to me though, looking at https://github.com/lsst/sims_maf/blob/master/python/lsst/sims/maf/metrics/crowdingMetric.py that the CrowdingMagUncertMetric is intended to accept a single magnitude, not a vector. Technically, it looks like you could set the 'rmag' to None, in which case the CrowdingMagUncertMetric would actually calll def _compCrowdError(self, magVector, lumFunc, seeing, singleMag=None): with singleMag set to None, in which case it would compute the crowding error for all the magnitudes in magVector (which would be the 'starMagBins') but then it would still just take the mean value of this, after it's returned to CrowdingMagUncertMetric. Would calling _compCrowdError directly address what you want to do?