Skip to content
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

draft nuv dark monitor. help. #156

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

draft nuv dark monitor. help. #156

wants to merge 3 commits into from

Conversation

cmagness
Copy link
Collaborator

@cmagness cmagness commented Feb 6, 2020

it doesn't break!!! but i think it's clunky and not sure what is happening at the plotting stage. everything seems to be fine up to there.

eventually will close #79
Resolves #157

@codecov
Copy link

codecov bot commented Feb 6, 2020

Codecov Report

Merging #156 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #156   +/-   ##
=======================================
  Coverage   92.68%   92.68%           
=======================================
  Files          18       18           
  Lines        1476     1476           
=======================================
  Hits         1368     1368           
  Misses        108      108           
Impacted Files Coverage Δ
cosmo/filesystem.py 88.38% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4bf5484...57ef9d2. Read the comment docs.

run = 'monthly'

def get_data(self):
# access data, perform any filtering required for analysis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# access data, perform any filtering required for analysis
"""access data, perform any filtering required for analysis"""

return data

def calculate_dark_rate(self, dataframe, xlim, ylim):
# calculate dark rate for one exposure, with a dataframe with TIME,
Copy link
Contributor

@jwhite3 jwhite3 Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# calculate dark rate for one exposure, with a dataframe with TIME,
"""calculate dark rate for one exposure, with a dataframe with TIME, XCORR, and YCORR values"""

# TODO: update docs
# docs = "https://spacetelescope.github.io/cosmo/monitors.html#<darks>"
output = COS_MONITORING

Copy link
Contributor

@jwhite3 jwhite3 Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Plot stuff
x = 'TIME'
y= 'DARK_RATE'
plot_type = 'scatter'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^ If you want to use the built-in basic plotting

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jwhite3 jwhite3 added this to the Add Dark Monitors milestone Feb 14, 2020
cosmo/monitors/nuv_dark_monitor.py Show resolved Hide resolved
cosmo/monitors/nuv_dark_monitor.py Outdated Show resolved Hide resolved
cosmo/monitors/nuv_dark_monitor.py Show resolved Hide resolved
cosmo/monitors/nuv_dark_monitor.py Outdated Show resolved Hide resolved
@@ -71,3 +71,7 @@ target/

# Finder
.DS_Store

# Misc
testoutputs/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this something for you in particular? If so, I'd be careful about adding it here since .gitignore could grow quite a bit depending on what folks are doing.

Instead, try using the the unit tests! You could even update some of the "cleanup" fixtures to remove "output" on the start of tests rather than the end so that you can inspect the test artifacts. If you'd like some suggestions or help with this, let me know!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, actually that would be helpful! but i was planning to remove that stuff before merging it in, it's just for ease of directory access right now

class NUVDarkDataModel(BaseDataModel):
"""Datamodel for NUV Dark files."""
files_source = FILES_SOURCE
subdir_pattern = '?????'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look like you're using subdir_pattern elsewhere, so I think there are a couple of options:

  1. Get rid of this attribute
  2. Use find_files (which uses subdir_pattern)

I'd recommend the 2nd option just because it might make "forward compatibility" easier (i.e. if dark datasets aren't manually identified in this way in the future). I think @dzhuliya has an example of using find_files on the fuv dark branch

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes i want to use find_files but for some reason it wasn't working for me so i was planning to update that. i'll look at dzhuliya's branch


return dark_rate_array, dec_year_array

def track(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The track method should return some sort of quantity (scalar, dataframe, etc), not execute plotting

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep that is the end goal, don't worry. once the histogram is working correctly it will get the rates from the histogram and return those

Comment on lines +135 to +191
fig = plt.figure(figsize=(12, 9))
bin_size = 1e-8
n_bins = int((dark_counts.max() - dark_counts.min()) / bin_size)
ax = fig.add_subplot(2, 1, 1)
ax.hist(dark_counts, bins=n_bins, align='mid', histtype='stepfilled')
counts, bins = np.histogram(dark_counts, bins=100)
cuml_dist = np.cumsum(counts)
count_99 = abs(cuml_dist / float(cuml_dist.max()) - .99).argmin()
count_95 = abs(cuml_dist / float(cuml_dist.max()) - .95).argmin()

mean = dark_counts.mean()
med = np.median(dark_counts)
std = dark_counts.std()
mean_obj = ax.axvline(x=mean, lw=2, ls='--', color='r', label='Mean ')
med_obj = ax.axvline(x=med, lw=2, ls='-', color='r', label='Median')
two_sig = ax.axvline(x=med + (2 * std), lw=2, ls='-', color='gold')
three_sig = ax.axvline(x=med + (3 * std), lw=2, ls='-',
color='DarkOrange')
dist_95 = ax.axvline(x=bins[count_95], lw=2, ls='-',
color='LightGreen')
dist_99 = ax.axvline(x=bins[count_99], lw=2, ls='-', color='DarkGreen')

ax.grid(True, which='both')
ax.set_title('Histogram of Dark Rates', fontsize=15, fontweight='bold')
ax.set_ylabel('Frequency', fontsize=15, fontweight='bold')
ax.set_xlabel('Counts/pix/sec', fontsize=15, fontweight='bold')
ax.set_xlim(dark_counts.min(), dark_counts.max())
ax.xaxis.set_major_formatter(FormatStrFormatter('%3.2e'))

ax = fig.add_subplot(2, 1, 2)
# log_bins = np.logspace(np.log10(dark.min()), np.log10(dark.max()),
# 100)
ax.hist(dark_counts, bins=n_bins, align='mid', log=True,
histtype='stepfilled')

ax.axvline(x=mean, lw=2, ls='--', color='r', label='Mean')
ax.axvline(x=med, lw=2, ls='-', color='r', label='Median')
ax.axvline(x=med + (2 * std), lw=2, ls='-', color='gold')
ax.axvline(x=med + (3 * std), lw=2, ls='-', color='DarkOrange')
ax.axvline(x=bins[count_95], lw=2, ls='-', color='LightGreen')
ax.axvline(x=bins[count_99], lw=2, ls='-', color='DarkGreen')

# ax.set_xscale('log')
ax.grid(True, which='both')
ax.set_ylabel('Log Frequency', fontsize=15, fontweight='bold')
ax.set_xlabel('Counts/pix/sec', fontsize=15, fontweight='bold')
ax.set_xlim(dark_counts.min(), dark_counts.max())
ax.xaxis.set_major_formatter(FormatStrFormatter('%3.2e'))

fig.legend([med_obj, mean_obj, two_sig, three_sig, dist_95, dist_99],
['Median: {0:.2e}'.format(med),
'Mean: {0:.2e}'.format(mean),
r'2$\sigma$: {0:.2e}'.format(med + (2 * std)),
r'3$\sigma$: {0:.2e}'.format(med + (3 * std)),
r'95$\%$: {0:.2e}'.format(bins[count_95]),
r'99$\%$: {0:.2e}'.format(bins[count_99])], shadow=True,
numpoints=1, bbox_to_anchor=[0.8, 0.8])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to reiterate, plotting should not be in track, but also, plotly should be used for plotting if possible by creating traces and whatnot and then adding them to the figure attribute:

    def plot(self):
        ...  # Make plotly traces here; let's say you have a list called "traces"
        self.figure.add_traces(traces)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that is the end goal as well, but marc wanted the plots replicated for now

Comment on lines +128 to +132
for index, row in plotdf.iterrows():
all_dec_year = list(
itertools.chain(all_dec_year, row["DECIMAL_YEAR"]))
all_dark_rates = list(
itertools.chain(all_dark_rates, row["DARK_RATE"]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit might be what's causing the funkiness in your plots perhaps?

Comment on lines +195 to +196
if self.data is None:
self.data = self.get_data()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see where you're going with this, and it's not a bad idea, but the precedent with the other monitors has been to not enable "asynchronous" steps in the sense that if you want to call individual steps, you still need to call the other steps that it depends on.

If you enabled this here, it should be enabled for all of the other monitors as well so there's no confusion on expected behavior between monitors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create DataModel for Dark monitors Add NUV Dark Monitor(s)
2 participants