Open nonsk131 opened 8 years ago
Great, thanks-- I'm looking over this and have a few suggestions. I'll get back to you soon (have to go to a meeting now but will be back and then will go through this and send comments).
Actually, I'm going to be delayed a bit more, but I will get back to this hopefully later today.
OK, a few initial comments:
get_mag_leaf
function depends on a particular naming convention of each observed leaf, one that is not really enforced anywhere, so this is not a reliable way to get the magnitude from an ObsNode
. Instead, you should take advantage of the fact that you can access the observed magnitude via n.value[0]
, if n
is the node in question.K00177
, the example case I've been using to check things---the magnitude comparison can only be between the same instruments... that is, it makes no sense to compare a magnitude from one survey to that of another. For example, in K00177
, my default model definition assigns model nodes to four stars observed by Kraus, though only two of them are observed by Robo-AO. So in this case, define_models
needs to know what instrument is shared by all the chosen leaves, and compare those observed magnitudes.Anyway, this is a good start on a tough problem! I'll be thinking about this more; stay tuned! Feel free to use this implementation if it works for your current synthetic simulations, but our goal will be something a bit more robust.
Another solution that might be even more robust is just to have the _get_leaves
and select_leaves
functions to by default return leaves in some sort of ordered fashion, since this is currently what dictates the labels of the model nodes...
Or to just raise a warning if things end up out of order, and then have a separate method do the rearranging if desired?... I'm just brainstorming here, the best solution isn't obvious to me!
modify add_models() so that it assigns _0 to the brightest star in the system