Skip to content

hf-visual workflow#2

Open
RoboDoig wants to merge 17 commits intomainfrom
test-run
Open

hf-visual workflow#2
RoboDoig wants to merge 17 commits intomainfrom
test-run

Conversation

@RoboDoig
Copy link
Copy Markdown
Contributor

This is a PR to merge the working hf-visual prototype into the main branch of hf-visual.

For review:

  • Successful run of the workflow in an experiment (tomorrow)
  • Determine which parts of the workflow should be absorbed into the core acquisition / schema repos
  • Validate the logging and loading of data
  • Approach for 'dummy' timestamps, attached timestamps that are not true hardware timestamps
  • SerialDevice must expose Name as well as PortName acquisition#8
  • Review TODO annotations in workflow

Comment thread .bonsai/Bonsai.config
<Package id="System.Runtime.CompilerServices.Unsafe" version="6.1.0" />
<Package id="UclOpen.Core" version="0.1.3-dev0" />
<Package id="UclOpen.Devices" version="0.1.3-dev0" />
<Package id="UclOpen.Core" version="0.1.2-dev0" />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Downgraded package? In future we must have a way to lock the version of acquisition to rigs schemas.

Comment thread examples/rig.py
arduino=MatrixArduino(
port_name="COM4",
baud_rate=1000000,
new_line="\n",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should decide on a strategy for the actual values of parameters. It is convenient to populate these from the pydantic class definitions, but much of the user interface could, and perhaps should, be yaml. Regardless these parameters should be logged with every session and consequent set of data.

I would propose the following rules:
Pydantic schemas should only provide class definitions. Set parameters should only be used here if they are internal properties that are a fundamental part of the "module" for this rig. This may mean the COM port is appropriate to have here in the ground truth schema, but I worry that it makes it too hidden if for any reason you switch port.
Configurable values that are subject to change should live in a yaml, and only an example of that yaml is version controlled.
Calibrations and other, static configurations could be logged as JSON, and calibration workflows should output these JSONs. Again, probably only an example needs to be version controlled, or not at all.

This creates a seperation of things that are decided by a workflow or procedure, defined and logged in JSON, pydantic or XML, and those that are human user defined, which should always live in yaml. So the yaml docuemnt is the only neccessary entry point.

Therefore, the only thing that should need to be changed by a user for any experiment to be run is in yaml.

Comment thread examples/session.py
commit="",
repository_url="https://github.com/ucl-open/hf-visual"
repository_url="https://github.com/ucl-open/hf-visual",
logging_root_path="C:/Users/saleem_lab/Documents/GitHub/hf-visual/temp_data",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

see previous comment on setting parameters like this in python.

Comment thread examples/session.py
session = Experiment(
# TODO - versions, repo configs etc.
session = UclOpenSession(
workflow="main.bonsai",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This generalisation of using "main.bonsai" may not hold water in future, if we are to have multiple bonsai workflows representing different experiments in the same "rig" repository.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider removing outdated development packages

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nothng wrong with this, but I wonder if there is a benefit to defining this as a specific data-type in data-type.py.


from ucl_open_hf_visual import __semver__

# TODO - should be part of main package?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

agreed, synchQuad should be generic

from ucl_open_hf_visual import __semver__

# TODO - should be part of main package?
# TODO - should be able to define generic number of sync quads (e.g. for different screens)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

once synchQuad exists, n synch quads can be defined the same way as other devices in the rig.py for a given rig repo. This is as a Dictionary of key (or "name"), value (or SynchQuads) pairs. Potentially a synch quad should be linked to a stimulus or trigger...?

@@ -0,0 +1,7 @@
from ucl_open.rigs.experiment import Experiment

# TODO - should be part of ucl open rigs
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, potentially part of the Experiment class?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants