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

WIP: Re-prototype use of mica stats instead of sybase #257

Closed
wants to merge 14 commits into from

Conversation

jeanconn
Copy link
Contributor

No description provided.


guides = GUIDES[GUIDES['agasc_id'] == int(agasc_id)]

mags = acqs['mag_obs'][acqs['mag_obs'] != 0].tolist()
Copy link
Member

Choose a reason for hiding this comment

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

np.concatenate([x, y, ..]) is a good function for your toolbox and works well here.

Copy link
Contributor Author

@jeanconn jeanconn Oct 11, 2018

Choose a reason for hiding this comment

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

I started with np.concatenate. It didn't seem to work if either x or y was empty. I'll go back to its docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I must have just has something else wrong when I blamed np.concatenate


acqs = ACQS[ACQS['agasc_id'] == int(agasc_id)]
time = float(time)
acq_5y = acqs[(acqs['guide_tstart'] >= (time - (5 * 365 * 86400)))
Copy link
Member

Choose a reason for hiding this comment

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

Why the 5 year time limit? I think the two important pieces of information are:

  • Best estimate of observed mag
  • Whether actual p_acq is persistently very different from model p_acq, e.g. due to a nearby bright spoiler.

Did you have a different idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't "rethink" about this when re-implimenting this last night, and left this mismatch (5y for acq on noid from the original prototype, all data for guide) in this WIP.

For acq, what do you consider a useful way to measure whether p_acq is persistently very different from model p_acq?

Copy link
Member

Choose a reason for hiding this comment

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

The super-simple thing to do is just add all the p_acq and compare to the actual number that were acquired. If the probability model is applicable to the star then those values should match. For extreme cases it is easy, e.g. sum of p_acq is 19 (for e.g. 23 trials) and in fact only 3 acqs were successful.

The trick is in establishing confidence intervals for not-so-obvious cases. This is basically like summing bernoulli distributions, but I'm not sure offhand if there is a clean computational formalism. One could always do Monte-Carlo simulation with a modest number of trials (a few hundred) to find cases of "very difference from the model".


acqs = ACQS[(ACQS['agasc_id'] == int(agasc_id))
& (ACQS['guide_tstart'] < time)]
ok = (acqs['img_func'] == 'star') & (~acqs['ion_rad']) & (~acqs['sat_pix'])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like in Py3 all these string columns are now bytestrings so this match on 'star' doesn't work. Should try to fix this on the mica side.

In [23]: acqs['img_func']
Out[23]: 
array([b'star', b'NONE', b'star', b'star', b'star', b'NONE', b'star',
       b'star', b'star', b'star', b'star', b'star', b'star', b'star',
       b'star', b'star', b'star', b'star', b'star', b'star', b'star',
       b'star'], 
      dtype='|S7')

Copy link
Member

Choose a reason for hiding this comment

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

If you turn it into an astropy Table then the compare will work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wondering if I should do that on the mica side. Also wondering if we have other codes that are going to be unhappy with this out of pytables.

Copy link
Member

Choose a reason for hiding this comment

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

This is a key sticking point in Py3 migration, and one of the main drivers for astropy/astropy#5700. Takeaway is to use astropy Table when dealing with character data coming from HDF5.

Copy link
Member

Choose a reason for hiding this comment

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

Upstream fix to use Table in mica seems like the right answer. Probably that will break less code than not doing it. Maybe?? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😄 I should probably start to review which of our hdf5 files have character data.

@jeanconn jeanconn changed the base branch from py3 to master December 18, 2018 11:44
@jeanconn jeanconn added this to the 13.1 milestone Jan 16, 2019
@jeanconn jeanconn modified the milestones: 13.1, 13.3 Jun 12, 2019
@jeanconn
Copy link
Contributor Author

Though I'm not actually sure if we want to finish this up or do https://github.com/sot/ska-projects/issues/114

@taldcroft
Copy link
Member

I'm lost on what is happening with all the commits that seem unrelated to using mica stats instead of sybase.

@jeanconn
Copy link
Contributor Author

I don't think this PR needs detailed review, I think we just need a plan for how to use previous observation data to review a new observation (you started to form some ideas for acq in this thread) and then need to decide if that should happen in starcheck or via agasc supplement.

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

Successfully merging this pull request may close these issues.

2 participants