created a weighted scorer and started writing tests #243
Conversation
| scores = scorer.weighted_score_array(time_resolution=20 * u.minute) | ||
| assert all(scores[0] - (c[0] * .8 + c2[0] * .7)/1.5 | ||
| np.array_equal(np.round(scores[0], 10), np.round((c[0] * .8 + c2[0] * .7)/1.5, 10)) | ||
| assert np.array_equal(np.round(scores[1], 10), np.round(c[1], 10))''' |
There was a problem hiding this comment.
This is waiting for the changes to the rescale functions because I wrote the MoonIlluminationConstraint's non-boolean version using them.
There was a problem hiding this comment.
This works after rebasing.
| if constraint.boolean_constraint: | ||
| # add to the mask designating where the score is zero | ||
| zeros[i] &= applied_score | ||
| # TODO: make a default weight=1 and merge these |
There was a problem hiding this comment.
Should this be addressed before merge?
There was a problem hiding this comment.
Yes, but we need to decide if we want weighting to be part of the constraints package (add a kwarg to Constraint) or the scheduling package (constraint_weights kwarg for ObservingBlock and global_constraint_weights kwarg for Schedule). During the implementation of either method it would be very easy to set the default weight and fix this TODO.
| end = self.schedule.end_time | ||
| times = time_grid_from_range((start, end), time_resolution) | ||
| score_array = np.zeros((len(self.blocks), len(times))) | ||
| zeros = np.ones((len(self.blocks), len(times)), dtype=int) |
There was a problem hiding this comment.
What is zeros, which is actually ones?
There was a problem hiding this comment.
I should add some comments clarifying. If any constraint has a time where it has a value of zero, that gets stored in the zeros array, then the final scores get multiplied by the zeros array at the end. This preserves the intention that "if any constraint isn't satisfied the score becomes zero".
There was a problem hiding this comment.
Ah. Can you think of a better name than zeros? Maybe zeroed_constraints or something like that, with a comment explaining?
There was a problem hiding this comment.
I changed it to constraint_zeros in the next commit..
| zeros[i] &= applied_score | ||
| # TODO: make a default weight=1 and merge these | ||
| elif constraint.weight: | ||
| zeros[i][np.where(applied_score == 0)] = 0 |
There was a problem hiding this comment.
Do you need this np.where, or would the indexing work the same without it?
There was a problem hiding this comment.
The indexing works without it, I didn't realize that worked.
| elif constraint.weight: | ||
| zeros[i][np.where(applied_score == 0)] = 0 | ||
| weight = constraint.weight | ||
| weights[i] += weight |
There was a problem hiding this comment.
Is there a reason that you add the weight in rather than appending it directly to the weight list?
There was a problem hiding this comment.
The weight is per block (i.e. weights = [block 1 weight sum, block 2 weight sum ,...]) so each constraint for a given block adds its weight to that block's weight sum.
| skip_global.append(i) | ||
| global_score = constraint(self.observer, targets, times) | ||
| if constraint.boolean_constraint: | ||
| zeros &= 1-global_score |
There was a problem hiding this comment.
I have no idea why the 1- is there (might have been a relic from a previous zero-tracking method). It is fixed in the next commit.
|
Hey @kvyh, I'm not sure why this PR is still hanging around and not merged – was there anything left to do on this one, or shall I merge it? |
|
@bmorris3 - Could we start with the bugfixes, to make sure new features do still work as expected? The green status here is misleading as it was green many month ago before things started to brake. |
| scores = scorer.weighted_score_array(time_resolution=20 * u.minute) | ||
| assert all(scores[0] - (c[0] * .8 + c2[0] * .7)/1.5) | ||
| np.array_equal(np.round(scores[0], 10), np.round((c[0] * .8 + c2[0] * .7)/1.5, 10)) | ||
| assert np.array_equal(np.round(scores[1], 10), np.round(c[1], 10)) |
There was a problem hiding this comment.
@kvyh Hello, I just started an internship with the goal of creating an optimized scheduler for astronomic observations with astroplan. I am interested in adding weight to the various constraints and I just found this post but this was 6 years ago and not implemented , I was wondering if this methode of adding weight works ?
I created a weighted scorer that scores by running (in essense):