-
Notifications
You must be signed in to change notification settings - Fork 0
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
DM-41027: Fix potential infinite loop in analysis_drp colorColorFitPlot #73
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import astropy.units as u | ||
import matplotlib | ||
import matplotlib.pyplot as plt | ||
import numpy as np | ||
|
@@ -106,6 +107,13 @@ class ColorColorFitPlotConfig(pipeBase.PipelineTaskConfig, | |
optional=True, | ||
) | ||
|
||
minPointsForFit = pexConfig.RangeField( | ||
doc="Minimum number of valid objects to bother attempting a fit.", | ||
dtype=int, | ||
default=5, | ||
min=1, | ||
) | ||
|
||
def setDefaults(self): | ||
super().setDefaults() | ||
self.axisActions.xAction.magDiff.returnMillimags = False | ||
|
@@ -205,16 +213,33 @@ def run(self, catPlot, dataId, runName, tableName, bands, plotName): | |
ys = plotDf[self.config.axisLabels["y"]].values | ||
|
||
plotInfo = parsePlotInfo(dataId, runName, tableName, bands, plotName, SN, SNFlux) | ||
if len(plotDf) == 0: | ||
if len(plotDf) < self.config.minPointsForFit: | ||
fig = plt.Figure() | ||
noDataText = ("No data to plot after selectors applied\n(do you have all three of " | ||
"the bands required: {}?)".format(bands)) | ||
if len(plotDf) == 0: | ||
noDataText = ("No data to plot after selectors applied\n(do you have all three of " | ||
"the bands required: {}?)".format(bands)) | ||
else: | ||
noDataText = ("Not enough data ({} < config.minPointsForFIt = {}) after selectors for " | ||
"fit.".format(len(plotDf), self.config.minPointsForFit)) | ||
fig.text(0.5, 0.5, noDataText, ha="center", va="center") | ||
fig = addPlotInfo(fig, plotInfo) | ||
else: | ||
fitParams = stellarLocusFit(xs, ys, self.config.stellarLocusFitDict) | ||
fig = self.colorColorFitPlot(plotDf, plotInfo, fitParams) | ||
|
||
try: | ||
fig = self.colorColorFitPlot(plotDf, plotInfo, fitParams) | ||
if len(fitParams["fitPoints"]) < 1: | ||
raise ValueError("No fitPoints for {}".format(dataId)) | ||
except ValueError as e: | ||
self.log.warning("Fit failed for %s with: %s", dataId, e) | ||
fig = plt.Figure() | ||
eStr = e.args[0] | ||
chunks, chunk_size = len(eStr), 50 | ||
eStrChunked = "" | ||
for i in range(0, chunks, chunk_size): | ||
eStrChunked += eStr[i:i+chunk_size] + "\n" | ||
noDataText = "Fit failed with:\n{}".format(eStrChunked) | ||
fig.text(0.5, 0.5, noDataText, ha="center", va="center") | ||
|
||
fig = addPlotInfo(fig, plotInfo) | ||
return pipeBase.Struct(colorColorFitPlot=fig) | ||
|
||
def colorColorFitPlot(self, catPlot, plotInfo, fitParams): | ||
|
@@ -272,10 +297,10 @@ def colorColorFitPlot(self, catPlot, plotInfo, fitParams): | |
the fit line is given in a histogram in the second panel. | ||
""" | ||
|
||
self.log.info(("Plotting %s: the values of %s against %s on a color-color plot with the area " | ||
"used for calculating the stellar locus fits marked.", | ||
self.config.connections.plotName, self.config.axisLabels["x"], | ||
self.config.axisLabels["y"])) | ||
self.log.info("Plotting %s: the values of %s against %s on a color-color plot with the area " | ||
"used for calculating the stellar locus fits marked.", | ||
self.config.connections.plotName, self.config.axisLabels["x"], | ||
self.config.axisLabels["y"]) | ||
|
||
# Define a new colormap | ||
newBlues = mkColormap(["darkblue", "paleturquoise"]) | ||
|
@@ -290,7 +315,10 @@ def colorColorFitPlot(self, catPlot, plotInfo, fitParams): | |
ys = catPlot[self.config.axisLabels["y"]].values | ||
mags = catPlot[self.config.axisLabels["mag"]].values | ||
|
||
if len(xs) == 0 or len(ys) == 0: | ||
if len(xs) < self.config.minPointsForFit or len(ys) < self.config.minPointsForFit: | ||
noDataText = ("Number of objects after cuts ({}) is less than the minimum\nrequired " | ||
"by config.minPointsForFit ({})".format(len(xs), self.config.minPointsForFit)) | ||
fig.text(0.5, 0.5, noDataText, ha="center", va="center") | ||
return fig | ||
|
||
# Points used for the fit | ||
|
@@ -305,10 +333,11 @@ def colorColorFitPlot(self, catPlot, plotInfo, fitParams): | |
SNsUsed = (catPlot[SNBand + "_" + plotInfo["SNFlux"]].values[fitPoints] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible for SNsUsed to be empty? I imagine the fit likely would have failed in that case, but if there's a chance it would keep going and result in an infinite loop again, maybe an explicit check would be worthwhile. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another good idea. I put in a check on
|
||
/ catPlot[SNBand + "_" + plotInfo["SNFlux"] + "Err"].values[fitPoints]) | ||
minSnUsed = np.nanmin(SNsUsed) | ||
magsUsed = mags[fitPoints] | ||
fluxesUsed = catPlot[SNBand + "_" + self.config.fluxTypeForColor].values[fitPoints] | ||
magsUsed = (fluxesUsed*u.nJy).to_value(u.ABmag) | ||
incr = 5.0 | ||
idsUsed = (SNsUsed < minSnUsed + incr) | ||
while sum(idsUsed) < max(0.005*len(idsUsed), 3): | ||
while sum(idsUsed) < max(int(0.005*len(idsUsed)), 1): | ||
incr += 5.0 | ||
idsUsed = (SNsUsed < plotInfo["SN"] + incr) | ||
medMagUsed = np.nanmedian(magsUsed[idsUsed]) | ||
|
@@ -331,11 +360,18 @@ def colorColorFitPlot(self, catPlot, plotInfo, fitParams): | |
ax.text(0.04, yLoc, infoText, color="C0", transform=ax.transAxes, | ||
fontsize=6, va="center") | ||
|
||
# Calculate the density of the points | ||
# Calculate the density of the points. Set all to 0.5 if density | ||
# can't be calculated. | ||
xyUsed = np.vstack([xs[fitPoints], ys[fitPoints]]) | ||
xyNotUsed = np.vstack([xs[~fitPoints], ys[~fitPoints]]) | ||
zUsed = scipy.stats.gaussian_kde(xyUsed)(xyUsed) | ||
zNotUsed = scipy.stats.gaussian_kde(xyNotUsed)(xyNotUsed) | ||
try: | ||
zUsed = scipy.stats.gaussian_kde(xyUsed)(xyUsed) | ||
except np.linalg.LinAlgError: | ||
zUsed = [0.5]*len(xyUsed) | ||
try: | ||
zNotUsed = scipy.stats.gaussian_kde(xyNotUsed)(xyNotUsed) | ||
except np.linalg.LinAlgError: | ||
zNotUsed = [0.5]*len(xyNotUsed) | ||
|
||
notUsedScatter = ax.scatter(xs[~fitPoints], ys[~fitPoints], c=zNotUsed, cmap=newGrays, | ||
s=0.3) | ||
|
@@ -350,6 +386,7 @@ def colorColorFitPlot(self, catPlot, plotInfo, fitParams): | |
ha="center", va="center", fontsize=7) | ||
cbText.set_path_effects([pathEffects.Stroke(linewidth=1.5, foreground="w"), pathEffects.Normal()]) | ||
cbAx.set_xticks([np.min(zUsed), np.max(zUsed)], labels=["Less", "More"], fontsize=7) | ||
|
||
cbAxNotUsed = fig.add_axes([0.12, 0.11, 0.43, 0.04]) | ||
plt.colorbar(notUsedScatter, cax=cbAxNotUsed, orientation="horizontal") | ||
cbText = cbAxNotUsed.text(0.5, 0.5, "Number Density (not used in fit)", color="k", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be validated as > 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. I made it a
RangeField
and gave itmin=1
.