DM-35336: Several updates and fixes to stellar locus plots #48
Conversation
laurenam
force-pushed
the
tickets/DM-35336
branch
4 times, most recently
from
06cde80
to
2c5586a
Compare
| The y maximum of the box used in the fit. | ||
| ``"fitPoints"`` | ||
| A boolean array indicating which points were usee in the | ||
| final ODR fit (`numpy.ndarray` [`bool`]). |
| # fit. | ||
| for SNBand in self.config.selectorActions.catSnSelector.bands: | ||
| SNsUsed = (catPlot[SNBand + "_" + plotInfo["SNFlux"]].values[fitPoints] | ||
| / catPlot[SNBand + "_" + plotInfo["SNFlux"] + "Err"].values[fitPoints]) |
There was a problem hiding this comment.
When this gets ported to analysis_tools, there might be functions in place to do this.
| infoText += r"$\geq$" + "{:0.1f} [".format(SNsUsedDict[SNBand]["minSnUsed"]) | ||
| infoText += r"$\lesssim$ " + "{:0.1f} mag]".format(SNsUsedDict[SNBand]["medMagUsed"]) | ||
| ax.text(0.04, yLoc, infoText, color="C0", transform=ax.transAxes, | ||
| fontsize=6, va="center") |
There was a problem hiding this comment.
I don't like the text and will try to come up with something neater for the future.
| if np.fabs(fitParams["mHW"]) > 1: | ||
| xMid = (fitParams["yMin"] - fitParams["bODR2"])/fitParams["mODR2"] | ||
| if np.fabs(mHW) > 1: | ||
| xMid = (yMin - fitParams["bODR"])/fitParams["mODR"] | ||
| xs = np.array([xMid - 0.5, xMid, xMid + 0.5]) |
There was a problem hiding this comment.
Should this be ODR rather than ODR2? Also below.
There was a problem hiding this comment.
Now I see the renaming in the other file, thanks git for making them in that order.
|
|
||
| alphas = [1.0, 0.5] | ||
| handles = [Rectangle((0, 0), 1, 1, color="none", ec="C0", alpha=a) for a in alphas] | ||
| labels = ["Refit", "HW"] | ||
| labels = ["ODR Fit", "HW"] |
There was a problem hiding this comment.
This should distinguish between the first round of fitting and the second.
There was a problem hiding this comment.
How so? I re-named Refit to ODR Fit because the former was never really distinguishing the difference between the first and second rounds of the fit (I was thinking we are only really distinguishing between the "Hard Wired" fit and our final fit on the plots. Suggestions other than ODR Fit welcome (e.g. Final Fit or just Fit)?
There was a problem hiding this comment.
I gather something is being fit even in the "hard wired", right? (Apologies for having forgotten the details of this process.) If so, how about "Fit" and "Fixed" followed by what is being fit or held fixed (e.g. "Slope")?
There was a problem hiding this comment.
Actually, there is nothing being fit "hard wired" wired case. It is meant to be a sort of "average/reference" fit so that one can identify any field-to-field variations in the location of the stellar locus (particularly in the "P1" principal component). We compute the mean and standard deviation from the hardwired fit coefficients; a shift in the former would indicate something is "different" in this field (a tract in the current usage), whether it be astrophysical due to a very different stellar population from the one used for the hardwired fit or something amiss with the data reduction. In the "fit" versions, the mean is ~zero by definition, so this information is lost (it is only the sigmaMad of the fit distribution that is the "metric" we typically look at).
I'm open to making a "hardwired" -> "fixed" name change, but wouldn't mind punting that to the ticket where this all gets implemented in analysis_tools (which did inherit the "hardwired" terminology). Is that reasonable?
There was a problem hiding this comment.
Yup, I'm happy to defer any changes to the analysis_tools port.
| @@ -385,15 +484,18 @@ def colorColorFitPlot(self, catPlot, plotInfo, fitParams): | |||
| minXs = -1*np.min(np.fabs(percsDists)) | |||
| maxXs = np.min(np.fabs(percsDists)) | |||
| plotPoints = ((dists < maxXs) & (dists > minXs)) | |||
| xs = np.array(dists)[plotPoints] | |||
| xs = np.array(dists)[plotPoints]*statsUnitScale | |||
There was a problem hiding this comment.
Probably the units should be accounted for in the distance calculation function.
There was a problem hiding this comment.
Oh, sure. I won't change the perpDistance function, but I'll do the scaling above where dists is computed.
laurenam
force-pushed
the
tickets/DM-35336
branch
2 times, most recently
from
54b4ec5
to
35ec253
Compare
| # in sync. This can (and should) be called in the pipeline definition | ||
| # if any of the inter-dependent configs are chaned (e.g. self.bands | ||
| # or self.fluxTypeForColor here. See plot_wFit_CModel in | ||
| # stellarLocusPlots.yaml for an example use case. |
There was a problem hiding this comment.
I like this way of setting inter-dependent configs, since I've wondered something similar for rho statistics. I have enforced that they are satisfied in a validate method, and I think there should be a validate method here as well, to make sure that the config doesn't become invalid by doing something incorrectly after calling. setConfigDependencies
There was a problem hiding this comment.
Heh...it felt totally ugly when I was writing it, but I couldn't think of a better way (I seem to recall slinking into Jim's office asking just how bad this idea was...yes, ugly, but not forbidden!)
There was a problem hiding this comment.
I don't even think it's ugly. We don't use this pattern as often as we should.
But I also agree with Arun's statement that should probably a validate method to check for anything where consistency is really required rather than just recommended, and it'd be nice to find a way to share logic between this setter and the validate implementation.
There was a problem hiding this comment.
Just to make sure I understand, are you suggesting this validate should happen in run (i.e. this is different than the Config class validate? I'm pretty sure I tried to make that work, but could never be sure it executed after all possible config manipulations...)?
There was a problem hiding this comment.
No, I'm just saying you should have a config validate that checks the invariants instead of relying only on this method having been called to set up the config. It's the job of whatever is running the task to call the config validate and then freeze the config.
There was a problem hiding this comment.
Ah, ok. I think I am again going to request that this be added in the ticket to update analysis_tools, since I would prefer not to have to do all the necessary "validation" (pun intended) of the validate function. Please push back if you don't think that's a reasonable request and want it added on this ticket!
There was a problem hiding this comment.
Definitely; the configuration will certainly have to change somewhat for that transition anyway.
| # The following config settings are conditional on other configs. | ||
| # Set them based on the inter-dependencies here to ensure they are | ||
| # in sync. This can (and should) be called in the pipeline definition | ||
| # if any of the inter-dependent configs are chaned (e.g. self.bands |
There was a problem hiding this comment.
| # if any of the inter-dependent configs are chaned (e.g. self.bands | |
| # if any of the inter-dependent configs are changed (e.g. self.bands |
| # in sync. This can (and should) be called in the pipeline definition | ||
| # if any of the inter-dependent configs are chaned (e.g. self.bands | ||
| # or self.fluxTypeForColor here. See plot_wFit_CModel in | ||
| # stellarLocusPlots.yaml for an example use case. |
There was a problem hiding this comment.
I don't even think it's ugly. We don't use this pattern as often as we should.
But I also agree with Arun's statement that should probably a validate method to check for anything where consistency is really required rather than just recommended, and it'd be nice to find a way to share logic between this setter and the validate implementation.
| @@ -3,6 +3,7 @@ | |||
| import matplotlib.patheffects as pathEffects | |||
| import numpy as np | |||
| import pandas as pd | |||
| import scipy.stats as scipyStats | |||
There was a problem hiding this comment.
This alias feels pretty unnecessary; it's just saving a single character.
|
|
||
| alphas = [1.0, 0.5] | ||
| handles = [Rectangle((0, 0), 1, 1, color="none", ec="C0", alpha=a) for a in alphas] | ||
| labels = ["Refit", "HW"] | ||
| labels = ["ODR Fit", "HW"] |
There was a problem hiding this comment.
I gather something is being fit even in the "hard wired", right? (Apologies for having forgotten the details of this process.) If so, how about "Fit" and "Fixed" followed by what is being fit or held fixed (e.g. "Slope")?
Includes adding configurable axis limit setting.
Adds config parameter, contourPlot to ColorColorPlotTask such that, when True, instead of a scatter plot color-mapped by magnitude, a number density based contour plot is created with two panels (left: stars, right: galaxies). These take some time to make so should only be generated for specific use cases or if runtime is not an issue (i.e. they are likely tier 2 or 3 plots).
This includes setting configs more consistently, fixing issues where the plot was misleading (i.e. not reflecting reality in terms of the points being used in the fit and the density color-mapping), updating the statistics to be given in mmag, updating labels to add info as well as remove some irrelevant entries, and adds the ability to set axis limits explicitly.
No description provided.