project-bluebird / dodo Goto Github PK

Although this seems to almost be complete in https://github.com/alan-turing-institute/dodo/pull/64 there appears to be errors relating how the travis script is configured.

This should be straight forward to alter such that tests are run using a docker-compose.yml file inspired from https://github.com/alan-turing-institute/twitcher/pull/8/files

If provided aircraft ID not in simulation, separation functions should return missing values in row/col for this aircraft:

• make this clear in specification
• implement in PyDodo

Already in rdodo.

See recent pull requests (e.g., #79 which only updates docstrings).

Get:

TypeError: attrib() got an unexpected keyword argument 'convert'

This seems to be a pytest error and might have to do with pytest version (as described here)

For example earth radius assumptions in GeoPy vs. geosphere

• make sure use the same values to get same results in PyDodo and rdodo

In recent builds from #79 and #78 a failure has occurred on Travis which prints:

Traceback (most recent call last):
  File "/home/travis/miniconda/envs/p36/bin/pytest", line 10, in <module>
    sys.exit(main())
  File "/home/travis/miniconda/envs/p36/lib/python3.6/site-packages/_pytest/config/__init__.py", line 61, in main
    config = _prepareconfig(args, plugins)
  File "/home/travis/miniconda/envs/p36/lib/python3.6/site-packages/_pytest/config/__init__.py", line 182, in _prepareconfig
    config = get_config()
  File "/home/travis/miniconda/envs/p36/lib/python3.6/site-packages/_pytest/config/__init__.py", line 156, in get_config
    pluginmanager.import_plugin(spec)
  File "/home/travis/miniconda/envs/p36/lib/python3.6/site-packages/_pytest/config/__init__.py", line 530, in import_plugin
    __import__(importspec)
  File "/home/travis/miniconda/envs/p36/lib/python3.6/site-packages/_pytest/tmpdir.py", line 25, in <module>
    class TempPathFactory(object):
  File "/home/travis/miniconda/envs/p36/lib/python3.6/site-packages/_pytest/tmpdir.py", line 35, in TempPathFactory
    lambda p: Path(os.path.abspath(six.text_type(p)))
TypeError: attrib() got an unexpected keyword argument 'convert'

It appears that pytest seems to have the package attrs as a dependency.

REF: https://stackoverflow.com/questions/58189683/typeerror-attrib-got-an-unexpected-keyword-argument-convert

New endpoints:

• EPLOG endpoint for pulling the logfile for the current episode
• SCENARIO endpoint for uploading a new scenario

The current implementations of all_positions and aircraft_positions return more columns than currently described in the specification document.

Check description of all functions.

Attempting to request a change in climb rate I get the following exception:

Exception: BlueBird returned '{
    "message": "Unknown arguments: vs"
}

Running the whole stack using the simurgh dockers. I found in the example scenario I was unable to change the vehicle speed with change_speed, the target vehicle would continue with the same speed, the command did not fail as no exception occurred. Likewise using the twitcher UI trying to change the speed also would not work.

I instanced a new vehicle using the dodo command, and again I found I was unable either via dodo or twitcher to change the vehicles speed.

The sector_exit command doesn't return any value...I would like to know if there is any issue with that?

When examining the issues relating to https://github.com/alan-turing-institute/dodo/pull/64 , I compared the method for
integration tests with what is being done in simurgh's travis script
This lead to the discovery of where it appeared to be issues with
load_scenario() function in bluebird

When running the integration tests locally using docker the following test fails:

...
tests/integration/test_manage_simulation.py::test_simulation_control PASSED
tests/integration/test_manage_simulation.py::test_load_bluesky FAILED
tests/integration/test_manage_simulation.py::test_load_fail PASSED
...

With the accompanying error of:

if http_error_msg:
>           raise HTTPError(http_error_msg, response=self)
E           requests.exceptions.HTTPError: 500 Server Error: INTERNAL SERVER ERROR for url: http://localhost:5001/api/v1/ic

It appears the test expects the file extension to be lowercase but is in fact
upper case as found in bluesky/scenario/8.SCN. A change to
tests/test_simulation_control.py should resolve this.

Please see this https://github.com/alan-turing-institute/nats/issues/98 where it outlines the fact that although this appears to be a general issue, it does not seem to arise when one does not use the docker builds

• Update get_position and all_positions functions to match changes in POS endpoint

  • endpoint includes info on sim_t - add as attribute to df

• Update load_scenario function

  • endpoint includes optional multiplier parameter

• Add new endpoints:

  • Direct to Waypoint (DIRECT)
  • List Route (LISTROUTE)

• Update Specification

tests/integration/test_flight_level.py::test_flight_level FAILED

The above test currently fails. The following is the output from the CI builds:

______________________________ test_flight_level _______________________________
    @pytest.mark.skipif(not bb_resp, reason="Can't connect to bluebird")
    def test_flight_level():
        cmd = reset_simulation()
        assert cmd == True
    
        resp = load_scenario("scenario/8.SCN")
        assert resp == True
    
>       assert current_flight_level("SCN1001") == 6096
tests/integration/test_flight_level.py:18: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pydodo/get_flight_level.py:96: in current_flight_level
    return get_flight_level(aircraft_id)['fl_current']
pydodo/get_flight_level.py:15: in get_flight_level
    resp.raise_for_status()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <Response [405]>
    def raise_for_status(self):
        """Raises stored :class:`HTTPError`, if one occurred."""
    
        http_error_msg = ''
        if isinstance(self.reason, bytes):
            # We attempt to decode utf-8 first because some servers
            # choose to localize their reason strings. If the string
            # isn't utf-8, we fall back to iso-8859-1 for all other
            # encodings. (See PR #3538)
            try:
                reason = self.reason.decode('utf-8')
            except UnicodeDecodeError:
                reason = self.reason.decode('iso-8859-1')
        else:
            reason = self.reason
    
        if 400 <= self.status_code < 500:
            http_error_msg = u'%s Client Error: %s for url: %s' % (self.status_code, reason, self.url)
    
        elif 500 <= self.status_code < 600:
            http_error_msg = u'%s Server Error: %s for url: %s' % (self.status_code, reason, self.url)
    
        if http_error_msg:
>           raise HTTPError(http_error_msg, response=self)
E           requests.exceptions.HTTPError: 405 Client Error: METHOD NOT ALLOWED for url: http://localhost:5001/api/v1/alt?acid=SCN1001

Requests to the unqualified bluebird url (by default http://0.0.0.0:5001/) get a 404 response in Bluebird 1.3.0, so an update is needed to the found_bluebird function.

• Download config.yml from master branch at install (rather than the py_dodo branch)
• Create config_param() function

Perhaps contained within an examples/ folder, a selection of notebooks that run both R and python kernels that showcase the functionality of dodo

Currently the Examples section in the docstrings do not contain working examples, this should be updated for (nearly) all functions.

This will help to link in with #66

Running the example file in simurgh, after some time and some change speed commands to the aircraft (which has it's own issue #114)

The ground speed in knots don't match between pydodo and twitcher:

the dodo spec says the speeds are in knts. The issue is there for both example vehicles.

User might need to specify host bluebird is running on (if other than localhost) and what simulator communicating with (default is bluesky).

Replace hard-coded strings (such as Bluebird endpoints ic, cre, etc.) with meaningful key-value configuration parameters. (See the config package.)

The host address is hard coded to 51.145.47.193
Agent are likely to use their 'localhost' address.
Either update the README to instruct the user to alter the address, or set the default host to 'localhost'.
i.e.
python3 run.py default_input_agents_read_scn.ini
will fail with:
HTTPConnectionPool(host='51.145.47.193', port=5002): Max retries exceeded with url: /api/v1/reset (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7f61cee12358>: Failed to establish a new connection: [Errno 110] Connection timed out',))

• simulation_step() doesn't work if no scenario has been loaded
  --> remove create_aircraft with load_scenario in all tests.

Remove hard coded strings.

Sorry I'm not sure if it belongs here or in Bluebird.

I created using create_aircraft a vehicle with a type "Phan4" from https://github.com/project-bluebird/bluesky/blob/master/data/performance/OpenAP/rotor/aircraft.json

The change_altitude command only seems to work in the simulation when the vehicle has some horizontal motion and the altitude remains fixed when the craft has a ground_speed of 0. How do a get a rotor craft behave more like a rotor craft and less like a fixed wing craft?

The issue are PyDodo integration tests. On some builds they all return 500 internal server error. This seems to suggest BlueSky might not be running and it is unclear why.

Is it possible asynchronous functions (the first integration tests to run) make BlueSky crash?

tests/integration/test_manage_simulation.py::test_simulation_control FAILED

The above test currently fails. The following is the output from the CI builds:

___________________________ test_simulation_control ____________________________
    @pytest.mark.skipif(not bb_resp, reason="Can't connect to bluebird")
    def test_simulation_control():
        """
        Test simulation endpoints
        - Pause Simulation
        - Resume Simulation
        - Reset Simulation
        - Simulation time
        - Simulator mode
        - Simulation step
        """
        aircraft_id = "TST1001"
        type = "B744"
        latitude = 0
        longitude = 0
        heading = 0
        altitude = None
        flight_level = 250
        speed = 200
    
        resp = reset_simulation()
        assert resp == True
    
        resp = create_aircraft(
            aircraft_id, type, latitude, longitude, heading, speed, altitude, flight_level
        )
        assert resp == True
    
        pos0 = aircraft_position(aircraft_id)
    
        resp = pause_simulation()
        assert resp == True
    
        pos1 = aircraft_position(aircraft_id)
        # check that position has changed since last position call
        assert pos1.loc[aircraft_id]["latitude"] > pos0.loc[aircraft_id]["latitude"]
    
        pos2 = aircraft_position(aircraft_id)
        # check that position has not changed since simulation was paused
        assert pos1.loc[aircraft_id]["latitude"] == pos2.loc[aircraft_id]["latitude"]
    
        resp = resume_simulation()
        assert resp == True
    
        pos3 = aircraft_position(aircraft_id)
        # check that position has changed since simulation was resumed
        assert pos3.loc[aircraft_id]["latitude"] > pos2.loc[aircraft_id]["latitude"]
    
        resp = set_simulation_rate_multiplier(1.5)
        assert resp == True
    
>       cmd = set_simulator_mode("agent")
tests/integration/test_manage_simulation.py:78: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pydodo/simulation_control.py:164: in set_simulator_mode
    return post_request(config_param("endpoint_simulator_mode"), body)
pydodo/utils.py:13: in post_request
    resp.raise_for_status()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <Response [404]>
    def raise_for_status(self):
        """Raises stored :class:`HTTPError`, if one occurred."""
    
        http_error_msg = ''
        if isinstance(self.reason, bytes):
            # We attempt to decode utf-8 first because some servers
            # choose to localize their reason strings. If the string
            # isn't utf-8, we fall back to iso-8859-1 for all other
            # encodings. (See PR #3538)
            try:
                reason = self.reason.decode('utf-8')
            except UnicodeDecodeError:
                reason = self.reason.decode('iso-8859-1')
        else:
            reason = self.reason
    
        if 400 <= self.status_code < 500:
            http_error_msg = u'%s Client Error: %s for url: %s' % (self.status_code, reason, self.url)
    
        elif 500 <= self.status_code < 600:
            http_error_msg = u'%s Server Error: %s for url: %s' % (self.status_code, reason, self.url)
    
        if http_error_msg:
>           raise HTTPError(http_error_msg, response=self)
E           requests.exceptions.HTTPError: 404 Client Error: NOT FOUND for url: http://localhost:5001/api/v1/simmode

In BlueBird, to use the step() function have to first:

• set SIMMODE to 'agent'
• set DTMULT to what want the step size to be

Only after that is it possible to call step().

Should dodo offer a single step function that executes all of these steps (or something along those line)?

By using Sphinx an auto generated set of static html files can be created such that one can showcase in detail the API of Pydodo as well as include links to relevant source code.

The would be contained inside a docs/ folder and served from master branch to be viewable via Github pages

Are get_position and aircraft_position within pydodo/request_position.py duplicates of one another. They seem to return the same values but appear to have slightly different implementations.

Change install instructions:
cd dodo/Pydodo
Change to:
cd dodo/PyDodo

Error raised when attempting to call functions in pydodo/get_flight_level.py.

After creating the following aircraft with:

>>> dodo.create_aircraft('BA1', 'A320', 10., 10., 10., 2000, 200)
True
>>> dodo.all_positions()
     type  altitude  ground_speed  latitude  longitude  vertical_speed
BA1  A320    196.85          1023  10.11094   10.01987               0

Then running a check on the flight level for example return this error:

>>> dodo.get_flight_level.current_flight_level('BA1')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/anaconda3/envs/nats/lib/python3.6/site-packages/pydodo/get_flight_level.py", line 96, in current_flight_level
    return get_flight_level(aircraft_id)['fl_current']
  File "/usr/local/anaconda3/envs/nats/lib/python3.6/site-packages/pydodo/get_flight_level.py", line 15, in get_flight_level
    resp.raise_for_status()
  File "/usr/local/anaconda3/envs/nats/lib/python3.6/site-packages/requests/models.py", line 940, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 405 Client Error: METHOD NOT ALLOWED for url: http://localhost:5001/api/v1/alt?acid=BA1

The same output occurs for both cleared_flight_level( .. ) and requested_flight_level( .. ) also.

Bluesky version:

v1.2.2 - docker image

Bluebird version:

v1.2.1 - docker image

How should dodo functions that wrap BlueBird API calls report error messages in case of failure?

Such reporting will be necessary for agents to decide how to react. For instance, the create_aircraft function returns a boolean value, with FALSE indicating that the attempt failed. However, there are various possible reasons for failure and the agent's next move may depend on the reason (e.g. it makes sense to retry in case of network error but not if the call failed because an aircraft with the given ID already exists). At the very least the agent will want to log the error message.

Should we instead throw an exception and include the error message returned by BlueBird in the HTTP response?

The listroute() function returns a data frame with waypoint names as row indexes. In PyDodo, these are currently sorted in alphabetical order (rather than the order in which they are meant to be passed on the route). Fix order to reflect route order.

Investigate whether this is also an issue in rdodo.