Refactor: Make agent default compatible with Bluesky Adaptive #86
Conversation
- Fix ask in bluesky - move original tell to base class, and add bluesky tell - refactor out update_models from base tell - refactor private training methods into base class, and their deps (essentially anything self. that wasn't decalred) - Add an unpack run that attempts learn's approach to pulling data, but will be tiled/dbv2 compat. Digestion is risky. - Add measurement plan that is QServer compat - Make bluesky relevat attrs into registered properties, and add server registrations
|
Should any of this CI be passing? I couldn't get any tests to pass on my main branch locally. Just want to sanity check. |
Based on this commit (49752b5) I believe it should be. |
There was a problem hiding this comment.
Most of it looks fine since its basically copy-pasted from the existing agent. I have some suggestions but nothing blocking.
I can approve once its been tested (and all other tests pass).
This package does not have great test coverage yet so I am a bit concerned about breaking changes.
| # Hope and pray that the digestion function designed for DataFrame can handle the XArray | ||
| data: pd.DataFrame = self.digestion(run.primary.data.read(), **self.digestion_kwargs) | ||
| return [data.loc[:, key] for key in self.dofs.names], [data.loc[:, key] for key in self.objectives.names] |
There was a problem hiding this comment.
Assuming this prayer can be answered by unit tests? Even with some mock data, if its simpler that way.
There was a problem hiding this comment.
Based on stepping back from the tiled/dbv2 requirements for the CI here, I'm going to hold on producing a test for this. Really trying to show a skeleton for how it can be done without breaking existing workflows.
We can definitely build up a smoke test, but I don't quite understand how data will be captured with the default plans using the queue server and tiled.
|
What I have learned in this process is:
|
Co-authored-by: Thomas Hopkins <thomashopkins000@gmail.com>
|
I realize I am not a contributor yet so I can't merge |
|
Hopefully this merge makes you a contributor. |
This does a lot of re-factoring, all with the goal of not augmenting existing functionality at all.
The Agent was split into a
BaseAgentthat it inherits from andAgent. Then aBlueskyAdaptiveAgentwas built that inherits from both thisBaseAgentandBlueskyAdaptiveBaseAgent, making as much use of the initial functionality as possible.In a nutshell, we needed to remove extra args and kwargs from the signature of data movement functions (ask/tell). The objects there, should be attributes of the Agent that can be adjusted via the REST API.
Other components of refactoring:
update_modelsfrom base tellself.in these functions that wasn't declared)unpack_runthat attemptslearn's approach to pulling data, but will be tiled/dbv2 compat. Digestion is risky, but I tried my best.