Horizon features #85
Horizon features #85
Conversation
requirements.txt
Outdated
| readline | ||
| gcloud | ||
| #readline | ||
| #gcloud |
There was a problem hiding this comment.
Actually you should leave these in here and just fix locally.
pocs/scheduler/constraint.py
Outdated
|
|
||
| def enter_coords(): | ||
|
|
||
| import ast |
There was a problem hiding this comment.
All import statements should be at the top of the file.
…/POCS into horizonFeatures
…a images) into the pocs examples notebooks folder.
brendan-o
force-pushed
the
horizonFeatures
branch
from
22c474a to
8b2e0f7
Compare
| super().__init__(*args, **kwargs) | ||
| self.obstruction_points = [] | ||
|
|
||
| print("Horizon.__init__") |
There was a problem hiding this comment.
Need to remove all print statements and use the self.logger.debug (or .info, .warning, etc) system.
| image_filename (file): The horizon panorama image to be processed | ||
| """ | ||
|
|
||
| from skimage.io import imread |
There was a problem hiding this comment.
import statements should always be at the top of the file
| If valid sets up a value for obstruction_points, otherwise leaves it empty | ||
| """ | ||
|
|
||
| from pocs.tests.test_horizon_limits import obstruction_points_valid |
| if not obstruction_points_valid(self.obstruction_points): | ||
| self.obstruction_points = [] | ||
|
|
||
| def interpolate(self, A, B, az): |
There was a problem hiding this comment.
Variable names should be lowercase and a and b are a little ambiguous since this is a tuple of points. Let's name them point_a and point_b or something more descriptive.
| prior_point = self.obstruction_points[0] | ||
| i = 1 | ||
| found = False | ||
| while(i < len(self.obstruction_points) and found is False): |
There was a problem hiding this comment.
while loops are generally used when you have continuous looping until some breaking condition. Here you have a set number of points you want to loop over so might as well loop them directly, something like:
for i, point in enumerate(self.obstruction_points):
if found:
break
....Then just use point instead of next_point assignment below.
| el = self.determine_el(az) | ||
|
|
||
| # Determine if the target altitude is above or below the determined minimum elevation for that azimuth | ||
| if alt - 7.5 > el: |
| veto2, score2 = h.get_score(time, observer, observation2) | ||
|
|
||
| assert veto1 is False and veto2 is False | ||
| assert score2 > score1 #Are these scores still relevant? How can I test scores for my class as its either 100 or 0? |
There was a problem hiding this comment.
I would just test one observation at at time and see if score1 > 0 or something. Depends on what you are actually trying to test. Let me know if that doesn't seem to make sense.
|
|
||
|
|
||
| # Unit tests for each of the evaluations | ||
| def test_validations(): |
There was a problem hiding this comment.
I'm a bit confused as to what is going on here. The validation_x subs should be given names that make sense. If validation_1 is testing that there is only one point it should be called test_single_point and a test_no_points_fail or something like that. Maybe we can sit down and go over a little bit of this in person.
| np.set_printoptions(threshold=np.nan) | ||
| print(edges1.astype(np.float)) | ||
|
|
||
| def get_config_coords(self): |
There was a problem hiding this comment.
This is reading the config file but not necessarily "getting" them since it doesn't return anything. I would rename to lookup_config_coords or something.
|
Hi Wilfred,
Sorry I only just saw this!
I am currently working on a summer vacation scholarship so I am
too busy right now to try fix these, hopefully I will be able to find some
time early next year for it!
…On Wed, Nov 8, 2017 at 4:45 PM, Wilfred Tyler Gee ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Sorry this took forever for me to get to. We'll want to merge this after
all the consolidation steps that are going on.
------------------------------
In pocs/scheduler/constraint.py
<#85 (comment)>:
> @@ -159,3 +165,160 @@ def get_score(self, time, observer, observation, **kwargs):
def __str__(self):
return "Moon Avoidance"
+
+
+class Horizon(BaseConstraint):
+
+ """ Implements horizon and obstruction limits"""
+
+ def __init__(self, *args, **kwargs):
+ super().__init__(*args, **kwargs)
+ self.obstruction_points = []
+
+ print("Horizon.__init__")
Need to remove all print statements and use the self.logger.debug (or
.info, .warning, etc) system.
------------------------------
In pocs/scheduler/constraint.py
<#85 (comment)>:
> + def process_image(self, image_filename):
+ """
+ Process the horizon_image to generate the obstruction_points list
+ Segment regions of high contrast using scikit image
+ Image Segmentation with Watershed Algorithm
+ bottom_left is a tuple, top_right is a tuple, each tuple has az, el
+ to allow for incomplete horizon images
+
+ Note:
+ Incomplete method, further work to be done to automate the horizon limits
+
+ Args:
+ image_filename (file): The horizon panorama image to be processed
+ """
+
+ from skimage.io import imread
import statements should always be at the top of the file
------------------------------
In pocs/scheduler/constraint.py
<#85 (comment)>:
> + binary = image > thresh
+
+ # Compute the Canny filter
+ edges1 = feature.canny(binary, low_threshold=0.1, high_threshold=0.5)
+
+ # Turn into array
+ np.set_printoptions(threshold=np.nan)
+ print(edges1.astype(np.float))
+
+ def get_config_coords(self):
+ """
+ Retrieves the coordinate list from the config file and validates it
+ If valid sets up a value for obstruction_points, otherwise leaves it empty
+ """
+
+ from pocs.tests.test_horizon_limits import obstruction_points_valid
Move to top.
------------------------------
In pocs/scheduler/constraint.py
<#85 (comment)>:
> +
+ def enter_coords(self):
+ """
+ Enters a coordinate list from the user and validates it
+ If valid sets up a value for obstruction_points, otherwise leaves it empty
+ """
+
+ from pocs.tests.test_horizon_limits import obstruction_points_valid
+ print("Enter a list of azimuth elevation tuples with increasing azimuths.")
+ print("For example (10,10), (20,20), (340,70), (350,80)")
+
+ self.obstruction_points = input()
+ if not obstruction_points_valid(self.obstruction_points):
+ self.obstruction_points = []
+
+ def interpolate(self, A, B, az):
Variable names should be lowercase and a and b are a little ambiguous
since this is a tuple of points. Let's name them point_a and point_b or
something more descriptive.
------------------------------
In pocs/scheduler/constraint.py
<#85 (comment)>:
> +
+ return el
+
+ def determine_el(self, az):
+ """
+ # Determine if the target altitude is above or below the determined minimum elevation for that azimuth
+
+ Args:
+ az (float or int): the target azimuth
+ """
+
+ el = 0
+ prior_point = self.obstruction_points[0]
+ i = 1
+ found = False
+ while(i < len(self.obstruction_points) and found is False):
while loops are generally used when you have continuous looping until
some breaking condition. Here you have a set number of points you want to
loop over so might as well loop them directly, something like:
for i, point in enumerate(self.obstruction_points):
if found:
break
....
Then just use point instead of next_point assignment below.
------------------------------
In pocs/scheduler/constraint.py
<#85 (comment)>:
> + prior_point = next_point
+ return el
+
+ def get_score(self, time, observer, observation, **kwargs):
+
+ target = observation.field
+ veto = False
+ score = self._score
+
+ az = observer.altaz(time, target=target).az
+ alt = observer.altaz(time, target=target).alt
+
+ el = self.determine_el(az)
+
+ # Determine if the target altitude is above or below the determined minimum elevation for that azimuth
+ if alt - 7.5 > el:
What is this magic number?
------------------------------
In pocs/tests/test_constraints.py
<#85 (comment)>:
> @@ -197,3 +197,30 @@ def test_moon_avoidance(observer):
assert veto1 is False and veto2 is False
assert score2 > score1
+
+def test_horizon(observer):
+ h = Horizon()
+
+ time = Time('2016-08-13 10:00:00')
+
+ observation1 = Observation(Field('HD189733', '20h00m43.7135s +22d42m39.0645s')) # HD189733
+ observation2 = Observation(Field('Hat-P-16', '00h38m17.59s +42d27m47.2s')) # Hat-P-16
+
+ veto1, score1 = h.get_score(time, observer, observation1)
+ veto2, score2 = h.get_score(time, observer, observation2)
+
+ assert veto1 is False and veto2 is False
+ assert score2 > score1 #Are these scores still relevant? How can I test scores for my class as its either 100 or 0?
I would just test one observation at at time and see if score1 > 0 or
something. Depends on what you are actually trying to test. Let me know if
that doesn't seem to make sense.
------------------------------
In pocs/tests/test_horizon_limits.py
<#85 (comment)>:
> + try:
+ validation_6(obstruction_points)
+ except AssertionError:
+ print("obstruction_points azimuth's should be in an increasing order")
+ v = False
+ try:
+ validation_7(obstruction_points)
+ except AssertionError:
+ print("obstruction_points should not consecutively have the same azimuth thrice or more")
+ v = False
+ assert v
+ return v
+
+
+# Unit tests for each of the evaluations
+def test_validations():
I'm a bit confused as to what is going on here. The validation_x subs
should be given names that make sense. If validation_1 is testing that
there is only one point it should be called test_single_point and a
test_no_points_fail or something like that. Maybe we can sit down and go
over a little bit of this in person.
------------------------------
In pocs/scheduler/constraint.py
<#85 (comment)>:
> + from skimage.filters import threshold_otsu
+ from skimage import feature
+ import numpy as np
+
+ image = imread(image_filename, flatten=True)
+ thresh = threshold_otsu(image)
+ binary = image > thresh
+
+ # Compute the Canny filter
+ edges1 = feature.canny(binary, low_threshold=0.1, high_threshold=0.5)
+
+ # Turn into array
+ np.set_printoptions(threshold=np.nan)
+ print(edges1.astype(np.float))
+
+ def get_config_coords(self):
This is reading the config file but not necessarily "getting" them since
it doesn't return anything. I would rename to lookup_config_coords or
something.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#85 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AWodAPG0Ze1mH5iVjqVWeaxH47ATF2y8ks5s0UAPgaJpZM4Mw-ns>
.
|
No problem. We have a number of changes to get integrated that will take some time. We got through most of our prep work for the 1.0.0 stream of things os early next year might be a good time for it. In the mean time we will try to test out what is there for Huntsman when we do the commissioning run. Enjoy the holidays! |
|
FYI, we should move forward with this. I need this for PAN006, which has a big obstruction in the north-west quadrant. |
Brendan changed a few other things and then emailed the changes to me instead of submitting to PR. I'll try to get those integrated soon but it will need additional work to get it going. |
|
Closing in favor of #368 |
#Adding the Horizon class