Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boostrap framework #7

Merged
merged 6 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added evolver/adapter/__init__.py
Empty file.
8 changes: 8 additions & 0 deletions evolver/adapter/demo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from evolver.adapter.interface import Adapter


class NoOpAdapter(Adapter):
ncalls = 0

def react(self):
self.ncalls += 1
14 changes: 14 additions & 0 deletions evolver/adapter/interface.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import pydantic
from abc import ABC, abstractmethod


class Adapter(ABC):
class Config(pydantic.BaseModel):
pass

def __init__(self, evolver, config: Config = None):
self.config = config or self.Config()

@abstractmethod
def react(self):
pass
53 changes: 49 additions & 4 deletions evolver/app/main.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,61 @@
import asyncio
from fastapi import FastAPI
from fastapi.responses import RedirectResponse

from evolver.device import Evolver, EvolverConfig
from .. import __project__, __version__


app = FastAPI()
evolver = Evolver()
Copy link
Member

Choose a reason for hiding this comment

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

Not really the best place for this comment, but before I forget - I think we should remove all usage of "evolver" from the code itself. All we have here is simple control loop type stuff and I see no reason to tie it to "evolver". Evolver can come in at the config or UI level. That way the users can see it etc, just doesn't need to be a code literal.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure. What else would we call it? This is for the evolver after all, no shame in that

Copy link
Member

Choose a reason for hiding this comment

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

I meant more of the attr, not the class, e.g., control_looper_thingy = Evolver(), at least in this exact instance, but like I said, not the most obvious place for me to make this comment and also my use of "all" was way too strong.



@app.get("/")
async def root():
return RedirectResponse(url='/docs')
async def describe_evolver():
return {
'config': evolver.config,
'state': evolver.state,
'last_read': evolver.last_read,
}


@app.get('/state')
async def get_state():
Copy link
Member

Choose a reason for hiding this comment

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

I think we'll need a set_state as well, so that stuff can be manually controlled via the web api.
This will be needed for the UI but also it would be great if multiple evolver servers can be controlled by another "main" server using the wbe api as the comms layer. Some self-similarity as mentioned before.

Copy link
Member Author

Choose a reason for hiding this comment

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

totally! just not here yet, and I think we need to ensure that the manual control and loop control don't happen together, as that could be confusing to user.

return {
'state': evolver.state,
'last_read': evolver.last_read,
}


@app.post("/")
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it's a good idea to post to root. Might be better/safer to have an explicit path to update the config.

Copy link
Member

Choose a reason for hiding this comment

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

I think we're going to need to think about locks of some sort since the control and update can happen from the web API and multiple users may be concurrently accessing - maybe they just can't by def of the lock. This may just need to be in the web API (hopefully), or lower (hopefully not much lower).

Copy link
Member

Choose a reason for hiding this comment

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

We're probably gonna have to bake into web api/ui some notion of the above even for a single user since the API is async and inpatient (or not) sequential posts have no guarantee of order. E.g., not being able to execute some commands (path funcs) if the system is not in a ready state to do so.

async def update_evolver(config: EvolverConfig):
Copy link
Member

Choose a reason for hiding this comment

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

We might need to think about when this allowed, i.e., always, only once at the beginning, or...?
Also, how this is synced if exec mid loop, though that logic wouldn't live here but in update_config (if it doesn't already).

Copy link
Member Author

Choose a reason for hiding this comment

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

I would think any time. There could be a problem mid loop I suppose in such a case (e.g. during the for adapter in self.adapters if self.adapters shrinks), but it could be resolved the next time around. We can see about this

evolver.update_config(config)


@app.get('/schema')
async def get_schema():
return evolver.schema


@app.get('/history/{name}')
async def get_history(name: str):
return evolver.history.get(name)


@app.get("/healthz")
async def healthz():
return {"message": f"Running '{__project__}' ver: '{__version__}'"}


async def evolver_async_loop():
while True:
evolver.loop_once()
Copy link
Member

Choose a reason for hiding this comment

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

Might add versatility to add pre_loop and post_loop methods, however, probably not here but to loop_once, or another layer so that loop_once can be used as the main loop method name:
E.g.,

def loop_once():
  self.pre_exec()
  self.exec()
  self.post_exec()

Perhaps even with chaining return values into the next call.

await asyncio.sleep(evolver.config.interval)


@app.on_event('startup')
async def start_evolver_loop():
asyncio.create_task(evolver_async_loop())


if __name__ == '__main__':
import uvicorn
uvicorn.run(app, host="127.0.0.1", port=8000, log_level="info")
1 change: 0 additions & 1 deletion evolver/app/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import pytest
from fastapi.testclient import TestClient

Copy link
Member

Choose a reason for hiding this comment

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

FYI PEP8 likes a line between each of the import kinds and that existing line separated 3rd party from local.

Imports should be grouped in the following order:

Standard library imports.
Related third party imports.
Local application/library specific imports.

You should put a blank line between each group of imports.

Copy link
Member Author

Choose a reason for hiding this comment

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

can we make ruff catch this?

Copy link
Member

@jamienoss jamienoss Mar 20, 2024

Choose a reason for hiding this comment

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

from ..main import app # Leave as relative for use in template: ssec-jhu/base-template.


Expand Down
19 changes: 19 additions & 0 deletions evolver/app/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,22 @@ def test_healthz(self, app_client):
assert response.status_code == 200
if __version__:
assert __version__ in response.json()["message"], response.json()

def test_evolver_app_default_config_dump_endpoint(self, app_client):
response = app_client.get('/')
assert response.status_code == 200
assert sorted(response.json().keys()) == ['config', 'last_read', 'state']

def test_evolver_update_config_endpoint(self, app_client):
data = {'hardware': {'test': {'driver': 'evolver.hardware.demo.NoOpSensorDriver'}}}
response = app_client.post('/', json=data)
assert response.status_code == 200
newconfig = app_client.get('/').json()['config']
assert newconfig['hardware']['test']['driver'] == 'evolver.hardware.demo.NoOpSensorDriver'

def test_evolver_app_react_loop_setup(self, app_client):
# The context manager ensures that startup event loop is called
# TODO: check results generated in react (may require hardware at startup, or forced execution of loop)
with app_client as client:
response = client.get('/')
assert response.status_code == 200
133 changes: 133 additions & 0 deletions evolver/device.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import time
import pydantic
from evolver.util import load_class_fqcn
from evolver.hardware.interface import SensorDriver, EffectorDriver
from evolver.serial import EvolverSerialUART
from evolver.history import HistoryServer


DEFAULT_SERIAL = EvolverSerialUART
DEFAULT_HISTORY = HistoryServer


class AdapterDescriptor(pydantic.BaseModel):
driver: str
config: dict = {}

def driver_from_descriptor(self, evolver):
cls = load_class_fqcn(self.driver)
conf = cls.Config.model_validate(self.config)
return cls(evolver, conf)


class HardwareDriverDescriptor(AdapterDescriptor):
calibrator: AdapterDescriptor = None
Copy link
Member

Choose a reason for hiding this comment

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

Ok, this might be the right layer for calibrators.



class EvolverConfig(pydantic.BaseModel):
vials: list = list(range(16))
hardware: dict[str, HardwareDriverDescriptor] = {}
adapters: list[AdapterDescriptor] = []
Comment on lines +29 to +30
Copy link
Member

Choose a reason for hiding this comment

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

Discussion point for tomorrow, in case I forget.

serial: AdapterDescriptor = None
Copy link
Member

Choose a reason for hiding this comment

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

serial -> comms (another discussion point)

history: AdapterDescriptor = None
enable_react: bool = True
enable_commit: bool = True
interval: int = 20


class Evolver:
hardware = {}
adapters = []
last_read = {}
Comment on lines +39 to +41
Copy link
Member

Choose a reason for hiding this comment

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

Are these meant to be class attrs - is this ok? Might be better to use a singleton as these look a little dangerous as is.


def __init__(self, config: EvolverConfig = EvolverConfig()):
self.update_config(config)

def update_config(self, config: EvolverConfig):
self.config = config
for name, driver in config.hardware.items():
self.setup_driver(name, driver)
for name in list(self.hardware.keys()):
if name not in config.hardware.keys():
del(self.hardware[name])
del(self.last_read[name])
Comment on lines +50 to +53
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just nuke self.hardware before looping over setup_driver?

"update" might also not be entirely accurate for this kind of set-del, perhaps "configure" and/or "reconfigure"? Not saying that there isn't a place for an "update", just that an "update" might be expected to operate similarly to a dict.update.

self.adapters = []
for adapter in config.adapters:
self.setup_adapter(adapter)
Comment on lines +55 to +56
Copy link
Member

Choose a reason for hiding this comment

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

FYI, I'm still confused about what's hardware vs adapter? Is the hardware the box and adapters those devices attached to the box?

if config.serial is not None:
self.serial = config.serial.driver_from_descriptor(self)
else:
self.serial = DEFAULT_SERIAL()
if config.history is not None:
self.history = config.history.driver_from_descriptor(self)
else:
self.history = DEFAULT_HISTORY()

def setup_driver(self, name, driver_config: HardwareDriverDescriptor):
driver_class = load_class_fqcn(driver_config.driver)
config = driver_class.Config.model_validate(driver_config.config)
calibrator = None
if driver_config.calibrator is not None:
calibrator = driver_config.calibrator.driver_from_descriptor(self)
self.hardware[name] = driver_class(self, config, calibrator)
self.last_read[name] = -1

def setup_adapter(self, adapter):
self.adapters.append(adapter.driver_from_descriptor(self))

def get_hardware(self, name):
return self.hardware[name]
Comment on lines +78 to +79
Copy link
Member

Choose a reason for hiding this comment

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

Needed?


@property
def sensors(self):
return {k: v for k,v in self.hardware.items() if isinstance(v, SensorDriver)}

@property
def effectors(self):
return {k: v for k,v in self.hardware.items() if isinstance(v, EffectorDriver)}
Comment on lines +81 to +87
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it's beneficial to separate sensors and effectors this explicitly. Perhaps at some higher-level layer (which, this might actually be). Perhaps it would be better to just have devices derived from a base interface that has get/set or read/write methods.


@property
def calibration_status(self):
Copy link
Member

Choose a reason for hiding this comment

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

Discussion point.

  • calibration as specific method(s)
  • per device and/or conglomerate over all (inter-device)?

return {name: device.calibrator.status for name,device in self.hardware.items()}

@property
def state(self):
return {name: device.get() for name,device in self.sensors.items()}

@property
def schema(self):
Copy link
Member

Choose a reason for hiding this comment

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

What happens if a schema is requested whilst the config is being updated?

Looks like we'll need a mutex at some class level, i.e., the "testbed" or main controller/scheduler.

Copy link
Member

Choose a reason for hiding this comment

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

Context managers are useful for this.

hardware_schemas = []
for n, hw in self.hardware.items():
s = {'name': n, 'kind': str(type(hw)),'config': hw.Config.model_json_schema()}
if isinstance(hw, SensorDriver):
s['output'] = hw.Output.model_json_schema()
if isinstance(hw, EffectorDriver):
s['input'] = hw.Input.model_json_schema()
hardware_schemas.append(s)
return {
'hardware': hardware_schemas,
'adapters': [{'kind': str(type(a)), 'config': a.Config.model_json_schema()} for a in self.adapters],
}

def read_state(self):
for name, device in self.sensors.items():
device.read()
self.last_read[name] = time.time()
self.history.put(name, device.get())


def evaluate_adapters(self):
for adapter in self.adapters:
adapter.react()

def commit_proposals(self):
for device in self.effectors.values():
device.commit()

def loop_once(self):
self.read_state()
# for any hardware awaiting calibration, call calibration update method here
if self.config.enable_react:
self.evaluate_adapters()
if self.config.enable_commit:
self.commit_proposals()
Comment on lines +128 to +133
Copy link
Member

Choose a reason for hiding this comment

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

Since both the flags/switches and methods belong to the instance, this logic should happen within the methods themselves. Well, maybe at least commit_proposals. Can always add a force=False kwarg as a bypass.

Empty file added evolver/hardware/__init__.py
Empty file.
28 changes: 28 additions & 0 deletions evolver/hardware/demo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from copy import copy
from evolver.hardware.interface import SensorDriver, EffectorDriver, VialConfigBaseModel, BaseCalibrator


class NoOpSensorDriver(SensorDriver):
class Config(VialConfigBaseModel):
echo_raw: int = 1
echo_val: int = 2

def read(self):
self.outputs = {
i: self.Output(vial=i, raw=self.config.echo_raw, value=self.config.echo_val)
for i in self.config.vials
}

def get(self):
return self.outputs


class NoOpEffectorDriver(EffectorDriver):
def commit(self):
self.comitted = copy(self.proposal)


class NoOpCalibrator(BaseCalibrator):
@property
def status(self):
return True
80 changes: 80 additions & 0 deletions evolver/hardware/interface.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import pydantic
from abc import ABC, abstractmethod


class VialConfigBaseModel(pydantic.BaseModel):
vials: list[int] | None = None


class VialBaseModel(pydantic.BaseModel):
vial: int


class BaseCalibrator(ABC):
class Config(pydantic.BaseModel):
calibfile: str = None

def __init__(self, evovler = None, config: Config = Config()):
self.config = config

@property
@abstractmethod
def status(self):
pass


class HardwareDriver(ABC):
class Config(pydantic.BaseModel):
pass
calibrator = None

def __init__(self, evolver, config = None, calibrator = None):
self.evolver = evolver
self.reconfigure(config or self.Config())
if calibrator:
self.calibrator = calibrator

def reconfigure(self, config):
self.config = config


class VialHardwareDriver(HardwareDriver):
def reconfigure(self, config):
super().reconfigure(config)
if config.vials is None:
config.vials = self.evolver.config.vials
else:
if not set(config.vials).issubset(self.evolver.all_vials):
raise ValueError('invalid vials found in config')


class SensorDriver(VialHardwareDriver):
class Config(VialConfigBaseModel):
pass
class Output(VialBaseModel):
raw: int
value: float
outputs: dict[int, Output] = {}

def get(self) -> list[Output]:
return self.outputs

@abstractmethod
def read(self):
pass


class EffectorDriver(VialHardwareDriver):
class Config(VialConfigBaseModel):
pass
class Input(VialBaseModel):
value: float
proposal: dict[int, Input] = {}
committed: dict[int, Input] = {}

def set(self, input: Input):
self.proposal[input.vial] = input

@abstractmethod
def commit(self):
pass
28 changes: 28 additions & 0 deletions evolver/history.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import time
import pydantic
from abc import ABC, abstractmethod
from collections import defaultdict


class History(ABC):
class Config(pydantic.BaseModel):
pass

@abstractmethod
def put(self, name, data):
Copy link
Member

Choose a reason for hiding this comment

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

Does a single history instance manage all "names" or is name an instance.attr and another server hoist multiple history instances, similar to python logging. Perhaps we should just leverage the python logging framework for the history rather than roll our own?

pass

@abstractmethod
def get(self, query):
pass


class HistoryServer(History):
Copy link
Member

Choose a reason for hiding this comment

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

Ok, I see, but now there's no interface for a given history, just them all aka the server. I think utilizing python logging or something else, would be good here.

def __init__(self, config = None):
self.history = defaultdict(list)

def put(self, name, data):
self.history[name].append((time.time(), data))

def get(self, name):
return self.history[name]
Loading
Loading