boostrap framework #7

amitschang · 2024-02-26T20:52:24Z

Bootsrapping a framework for the application. The general principal in this proposal is that we have an evolver that contains a set of hardware and experiment adapters with associated configuration, and the main entrypoint into its useful work is a REPL (read-eval-progress (or control)-loop) loop function. The functionality from this is then made available via web api in a running application.

📓 Notes 📓 :

The pattern where we have pydantic models for config and inputs/outputs as class members is designed around making this available via fastapi, but in a dynamic way (e.g. the schema endpoint) - a generic UI could figure out how to build a form for anything here
The serial module could easily be a hardware, but since the default hardware will use this for the forseeable future, it makes sense to have this as first-class-citizen.
The experiment adapters didn't seem to me to fit well with hardware, so they are something different, but interfaced and configured similarly.
Probably some locations of these things need refactoring

🚧 Purposeful omissions 🏗️ :

this doesn't include anything that actually might do work, just laying out the framework
history is just to demonstrate idea, so lacks a lot
calibrations left out aside from some basic structures, this needs an some additional interfacing to enable tracking and feedback and support multiple steps for this.
tests aren't very comprehensive, but here to establish sanity
config from/to a file - should be added later

🎮 Try it out! 🎲

start application tox -e test exec -- python -m evolver.app.main
setup with dummy config curl -D- -Hcontent-type:application/json -XPOST localhost:8000/ -d '{"hardware": {"test": {"driver": "evolver.hardware.NoOpSensorDriver", "config": {"echo_val": 10}}}, "adapters": [{"driver": "evolver.adapter.NoOpAdapter"}]}'
can view docs via http://localhost:8000/docs
get the state curl localhost:8000/state (should update with the results of NoOpSensorDriver after a few seconds)

jamienoss · 2024-03-07T21:12:15Z

@amitschang 👍 The BSR deadline is next Thurs (3/14) so I need to concentrate on that. I'll take a look at this on Friday. Cool with you?

jamienoss

Great ground work on getting the foundations bootstrapped, thanks!

Since this is the foundation for further discussion etc, there's not much stopping this from being merged. The only points that I think might be useful to address pre-merge, so as to curb further confusion (mine), is to:

remove code from all __init__.py
possible also namespace pydantic.BaseModel

Let's get a miro board going tomorrow so we can design the necessary layers and their sdk/api starting with what we have in here.

BTW a lot of my comments are just notes and discussion points for us to design around tomorrow so I wouldn't necessarily waste time answering any of them here. Might be better to chat about stuff and then we can record consensus here after the fact etc - as it could get repetitive for you if I've just got the wrong end of the stick and a lot is just off. Sound like a good plan?

jamienoss · 2024-03-18T15:58:59Z

evolver/adapter/__init__.py

@@ -0,0 +1,21 @@
+from abc import ABC, abstractmethod


[revision] Is it cool if we move this out of the __init__.py, please? I think it's hard to find code of this significance when it's hidden in a __init__.py. It also doesn't help by creating indirect imports.

I think it would be cleaner and more findable if base interface classes went in their own module file, i.e., adapter.interface.py or something similar.

Same goes for hardware.__init__.py.

jamienoss · 2024-03-18T15:59:47Z

evolver/adapter/__init__.py

+from pydantic import BaseModel
+
+
+class Adapter(ABC):


What exactly is an adapter? Is this already used evolver terminology? Is this an adapter like a dongle's a hardware adapter?

we could call it Reactor or something. These are meant to do the control based on hardware values

jamienoss · 2024-03-18T16:04:29Z

evolver/hardware/__init__.py

+    vial: int
+
+
+class BaseCalibrator(ABC):


I think it would be better if we removed the notion of "calibration". Instead, I think it would be more generic if we added pre_<> and post_<> methods to something. This way, they can be used however desired, inc for calibration - we can just add something in the docstring stating as example that any calibration can be added here.

At a higher-level, i.e., "evolver" layer this is good, the above stmnt was meant for lower-level interface code, which I don't think this is.

Not sure how this would work. Lets go over the existing calibration routines and see

jamienoss · 2024-03-18T20:10:52Z

evolver/app/main.py

+
+
+@app.get('/state')
+async def get_state():


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.

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.

jamienoss · 2024-03-18T20:14:42Z

evolver/app/main.py

+
+
+@app.post("/")
+async def update_evolver(config: EvolverConfig):


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).

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

jamienoss · 2024-03-19T00:25:54Z

evolver/serial.py

+    kind: str = 'r'
+
+
+class Serial(ABC):


Since this is (currently) a very generic interface, I don't think we should call it "serial", it could be anything, maybe we use "connection"?

jamienoss · 2024-03-19T00:32:40Z

evolver/serial.py

+        # the addressed device is considered owner of the line.
+        with self.lock:
+            if self.serial is None:
+                self._connect()


I think the connection should be context managed. And probably not assigned to self.serial, might be better to assign a communication class but not an instance. Better still a partial so that it can be called without each call needing to pass the config.

I don't quite understand this. I don't think we need a bunch of calls that pass any config. There would generally be one of these per evolver (with the connection wired up to rpi within the box), and the adapters(reactors) call its communicate method (this needs locking). The connect within the lock has some advantage, but ought only be done on the first call

I just meant that the connection should be closed, i.e., context managed:

with self._connect() as serial: serial.write()

and the config ref was for the baud rate etc that gets passed to the connection in _connect().
Instead, it could be:

self.serial = serial.Serial or

self.serial = functools.partial(serial.Serial, port=self.config.port, baudrate=self.config.baudrate, timeout=self.config.timeout)

oh. Any reason it shouldn't hold the connection open for the lifetime?

jamienoss · 2024-03-19T00:42:04Z

evolver/serial.py

+        self.lock = Lock()
+
+    def _connect(self):
+        self.serial = serial.Serial(port=self.config.port, baudrate=self.config.baudrate, timeout=self.config.timeout)


If we lock on the connection's usage below we might want to lock here also for developer safety even if this is a protected method. That being said, it would have to be an RLock and it may not be able to be. We might just need to nuke this func altogether or just return it rather than assign it. Let's discuss.

jamienoss · 2024-03-19T01:03:42Z

evolver/serial.py

+            self.serial.write(self._encode_command(cmd))
+            response = self.serial.readline()
+            try:
+                data = self._parse_response(response)
+            except Exception:
+                raise ValueError('invalid response')
+            ack = cmd.copy(update=dict(kind='a', data=[b'' for i in cmd.data]))
+            self.serial.write(ack)


I have to refresh my memory but at first glance this looks odd, particularly sending an ack on the read whose only purpose is to check for the ack echo on the write - seems like an infinite hand shake, no?
I also can't remember if pyserial has the functionality to trivially handle this, I thought it did since we'd have to loop on a failed ack to re-receive dropped data etc, to handle this properly. Perhaps the hardware is doing something bespoke?

I think this is right, you can have a look at https://khalil-lab.gitbook.io/evolver/software/server-raspberry-pi and also https://github.com/FYNCH-BIO/evolver/blob/master/evolver/evolver_server.py#L252

jamienoss · 2024-03-19T01:09:03Z

evolver/util.py

+    return getattr(module, cls)
+
+
+def driver_from_descriptor(evolver, descriptor):


Looks like "descriptor" should be a class that this func belongs to. For example, this func presumes descriptor.driver exists as an attr.

this could be a method of AdapterDescriptor then

jamienoss

Thanks for moving stuff around 👍

This looks like an awesome foundation to merge in and work on.

The only major design points to address beyond this PR, as we discussed offline, are:

the need to move the "connection" attribute into the hardware/driver code for encapsulation so that a "device" can be communicated with without needing the evolver object. This will require a connection interface class. Add connection interface #19
and I'd like to take another mo to rethink the adapter class, does that have to only belong to the evolver/manager class or can it belong to the device class?

The above points are to make sure we have a self-similar architecture so that we/anyone can design stuff both top-down and bottom-up for complete modularity and extensibility. E.g., top down: single device class representing all 16 vials, or bottom-up: a single device instance representing a single vial (or per-vial-device) either as standalone or can be aggregated into a conglomerate device representing N other devices i.e., the evolver box (but without special casing the interface class for it - which is the crux).

amitschang · 2024-04-04T17:10:50Z

Cool thanks! I agree the per vial interface especially for experiment level makes a lot of sense, issue for that here: #20

amitschang force-pushed the layout branch from 3a9ff5f to a1c0a7a Compare February 26, 2024 20:55

amitschang added 4 commits March 5, 2024 10:26

wip

e3fe710

add serial and adapter

f46ab6b

Add schema, state endpoints

051301b

Add history

71c18d8

amitschang force-pushed the layout branch from a719b49 to 71c18d8 Compare March 6, 2024 17:28

amitschang changed the title ~~wip~~ boostrap framework Mar 7, 2024

amitschang marked this pull request as ready for review March 7, 2024 17:31

amitschang requested a review from a team as a code owner March 7, 2024 17:31

amitschang requested a review from jamienoss March 7, 2024 17:31

jamienoss assigned amitschang Mar 7, 2024

jamienoss reviewed Mar 19, 2024

View reviewed changes

amitschang added 2 commits March 21, 2024 15:04

refactor

785a2a8

refactor

0c99b68

amitschang requested a review from jamienoss April 3, 2024 19:06

jamienoss approved these changes Apr 3, 2024

View reviewed changes

amitschang merged commit 9316fc2 into main Apr 4, 2024
6 checks passed

amitschang mentioned this pull request Apr 4, 2024

Bug fix - move mutable class parameters #23

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

boostrap framework #7

boostrap framework #7

amitschang commented Feb 26, 2024 •

edited

Loading

jamienoss commented Mar 7, 2024

jamienoss left a comment •

edited

Loading

jamienoss Mar 18, 2024

jamienoss Mar 18, 2024

amitschang Mar 19, 2024

jamienoss Mar 18, 2024

amitschang Mar 19, 2024

jamienoss Mar 18, 2024

amitschang Mar 19, 2024

jamienoss Mar 18, 2024

amitschang Mar 19, 2024

jamienoss Mar 19, 2024

jamienoss Mar 19, 2024

amitschang Mar 19, 2024

jamienoss Mar 19, 2024

amitschang Mar 19, 2024

jamienoss Mar 19, 2024

jamienoss Mar 19, 2024

amitschang Mar 19, 2024

jamienoss Mar 19, 2024

amitschang Mar 19, 2024

jamienoss left a comment •

edited

Loading

amitschang commented Apr 4, 2024



		@app.post("/")
		async def update_evolver(config: EvolverConfig):

		return getattr(module, cls)


		def driver_from_descriptor(evolver, descriptor):

boostrap framework #7

boostrap framework #7

Conversation

amitschang commented Feb 26, 2024 • edited Loading

jamienoss commented Mar 7, 2024

jamienoss left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamienoss left a comment • edited Loading

Choose a reason for hiding this comment

amitschang commented Apr 4, 2024

amitschang commented Feb 26, 2024 •

edited

Loading

jamienoss left a comment •

edited

Loading

jamienoss left a comment •

edited

Loading