Repository navigation
Declare microvolts as the unit of the velocity systems' output - #13
Merged
Merged
Conversation
Velocity2Spike and Velocity2LFP now stamp attrs["unit"] = "microvolts" on every output message. The amplitude is set by the DNSS spike templates (microvolts) and the LFP scale is chosen to match, but nothing declared it, so consumers such as an LSL outlet could not report a unit. VelocityEncoder inherits the unit through Add and LineNoise without an extra stage. The stamping stage is private to the system package.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The velocity systems produce signals in microvolts — the DNSS spike templates (
wf_orig) are µV and the LFP noise scale is chosen to sit under them — but no message said so. Downstream consumers such as an LSL outlet, which writesattrsinto the stream description, therefore published nounit.dnss/wfs.py:WF_UNIT = "microvolts"alongsidewf_orig.system/_signal_unit.py(new, private): a transformer that setsattrs["unit"], preserving other attrs.Velocity2SpikeandVelocity2LFPend with that stage, so each declares the unit standalone.VelocityEncoderadds no stage: bothAddinputs carry the unit andLineNoisepreserves attrs. Docstrings now give µV for the output,drift_scaleandline_noise_amp.The unit is a constant, not a setting: the amplitudes are fixed in µV, so a configurable unit could only disagree with the data.
Tests
tests/unit/test_signal_unit.py: stamping, preserving other attrs, replacing a different unit.tests/integration/test_velocity_encoder_unit.py: Clock → Spiral → Diff →VelocityEncoder(line noise on); every output message hasunit == "microvolts".Full suite: 164 passed; ruff clean.
Note: on macOS 27 the locked scipy 1.15.3 fails to load (dyld
__thread_bsserror), so I ran the suite withuv run --with scipy==1.18.1. The lock is unchanged here; bumping it is a separate change.