Skip to content

two functions for rescaling#236

Merged
bmorris3 merged 13 commits intoastropy:masterfrom
kvyh:direction_minmax
Sep 19, 2016
Merged

two functions for rescaling#236
bmorris3 merged 13 commits intoastropy:masterfrom
kvyh:direction_minmax

Conversation

@kvyh
Copy link
Contributor

@kvyh kvyh commented Aug 17, 2016

This is an alternate form of #228, with different functions for when the score gets better or worse for larger values. Would this be better than the single function in #228?

@kvyh
Copy link
Contributor Author

kvyh commented Aug 17, 2016

min_value, max_value and best_value, worst_value are both easy to understand, the issue I'm having is in naming the regions beyond the best and worst values. The options I've though of are: (greater_than, less_than) ; (above_max, below_min) ; (better_than , worse_than) ; (beyond_best , beyond_worst). The first two are for min,max and latter two are for best,worst.

@bmorris3
Copy link
Contributor

Would you add a note on this in the User-Defined Constraint tutorial?

above = rescaled > 1
rescaled[below] = 0
rescaled[above] = 1
def min_best_rescale(vals, min_val, max_val, greater_than=0, less_than=1):
Copy link
Member

Choose a reason for hiding this comment

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

How about naming the last two arguments (which might be just one if you make the changes I'm about to discuss in the comments) as "greater_score or similar? That makes it clearer that you're saying what the output is, rather than what input value is.

Similarly, perhaps you should use the word "score" in the docstring. E.g.

Rescales the input array ``vals`` to be a score (between zero and one), where ``min_val`` 
is the best and ``max_val`` is whatever ``greater_score`` is. 

@eteq
Copy link
Member

eteq commented Aug 18, 2016

This was discussed in a google hangout, but just to leave a note here on the topic: I'd suggest dropping the argument of whichever of greater_than/less_than has a default of 0. The reasoning here is that for a function named e.g. min_best_rescale, it won't ever make sense for values greater than max_val to be something other than 0. In contrast, less_than is still useful because sometimes you want values below that to be 0 but other times you might want it to be 1.

In general, though, I'm 👍 on having the two separate functions. Much easier to understand.

@kvyh
Copy link
Contributor Author

kvyh commented Aug 19, 2016

I changed the docstring. I still need to add it into the constraints tutorial.

@kvyh
Copy link
Contributor Author

kvyh commented Aug 20, 2016

I changed the constraint tutorial to contain a non-boolean constraint that uses the min_best_rescale

max_val : value
best value cared about (rescales to 1)
less_than_min : 0 or 1
what is returned for ``vals`` below min_val. (in some cases
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you put double ticks around min_val on this line?

@kvyh
Copy link
Contributor Author

kvyh commented Aug 23, 2016

I changed the parameter types to "array-like" and "float" and ``ed the min_val and max_val.

@bmorris3
Copy link
Contributor

Thanks @kvyh!

@bmorris3 bmorris3 merged commit 5c40add into astropy:master Sep 19, 2016
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.

3 participants