-
Notifications
You must be signed in to change notification settings - Fork 36
fix: compute Experiment start/end time as union across all devices #137
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
Merged
schewskone
merged 26 commits into
sensorium-competition:main
from
binary69:fix/experiment-global-time-range
Mar 20, 2026
Merged
Changes from 8 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
ec61bab
fix: compute Experiment start/end time as union across all devices
binary69 1008758
test: use pytest tmp_path for cleaner test isolation
binary69 118a928
style: auto-format with black and isort
invalid-email-address 36e6707
fix: add None guard for dev.start_time and end_time in _load_devices
binary69 45c6881
test: separate data creation helpers into create_experiment.py
binary69 38153ad
style: auto-format with black and isort
invalid-email-address 791bb98
fix: improve None handling with warning and add finite range validation
binary69 d6737de
style: auto-format with black and isort
invalid-email-address 7c07754
fix for pull request finding
binary69 dbf1896
fix for pull request finding
binary69 55e8f2e
style: auto-format with black and isort
invalid-email-address a068290
style: reformat elif condition to Black-compliant style
binary69 ae606bd
style: auto-format with black and isort
invalid-email-address 929c92d
fix: experiment time range validation and test refactor
binary69 a7edf85
fix: remove unused imports flagged by ruff
binary69 423f86f
style: auto-format with black and isort
invalid-email-address 86dc0bc
fix: replace np.isinf with np.isfinite to catch NaN and -inf values
binary69 3f4aa6e
fix: make sampling_rates and offsets per-device lists in make_modalit…
binary69 2e3cddf
fix: expand test coverage with edge cases, parametrize invalid device…
binary69 3c2ccc3
style: auto-format with black and isort
invalid-email-address 863e52c
fix: restore imports to match main branch
binary69 acbfd6c
style: auto-format with black and isort
invalid-email-address 17f752e
merge: sync with upstream main and resolve experiment.py conflict
binary69 33828ea
merge: resolve experiment.py conflict keeping upstream main imports
binary69 63f7589
fix: use correct sampling_rate parameter and random values in union test
binary69 2488615
style: auto-format with black and isort
invalid-email-address File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import numpy as np | ||
| import yaml | ||
|
|
||
|
|
||
|
binary69 marked this conversation as resolved.
|
||
| def make_sequence_device(root, name, start, end, sampling_rate=10.0): | ||
| """Create a single sequence device folder under root.""" | ||
| device_root = root / name | ||
| (device_root / "meta").mkdir(parents=True, exist_ok=True) | ||
|
pollytur marked this conversation as resolved.
Outdated
|
||
|
|
||
| n_samples = int((end - start) * sampling_rate) + 1 | ||
|
binary69 marked this conversation as resolved.
Outdated
|
||
| timestamps = np.linspace(start, end, n_samples) | ||
| data = np.random.rand(n_samples, 5) | ||
|
binary69 marked this conversation as resolved.
Outdated
|
||
|
|
||
| np.save(device_root / "timestamps.npy", timestamps) | ||
| np.save(device_root / "data.npy", data) | ||
|
|
||
| meta = { | ||
| "start_time": start, | ||
| "end_time": end, | ||
| "modality": "sequence", | ||
| "sampling_rate": sampling_rate, | ||
| "phase_shift_per_signal": False, | ||
| "is_mem_mapped": False, | ||
| "n_signals": 5, | ||
| "n_timestamps": n_samples, | ||
| "dtype": "float64", | ||
| } | ||
| with open(device_root / "meta.yml", "w") as f: | ||
| yaml.safe_dump(meta, f) | ||
|
|
||
|
|
||
| def make_modality_config(*device_names): | ||
| return { | ||
| name: {"interpolation": {"sampling_rate": 10.0, "offset": 0.0}} | ||
|
binary69 marked this conversation as resolved.
Outdated
|
||
| for name in device_names | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import pytest | ||
|
|
||
| from experanto.experiment import Experiment | ||
|
|
||
| from .create_experiment import make_modality_config, make_sequence_device | ||
|
|
||
|
|
||
| def test_experiment_start_end_time_reflects_union(tmp_path): | ||
| """ | ||
| Experiment.start_time and end_time should reflect the union of all | ||
| device time ranges — earliest start and latest end across all devices. | ||
| """ | ||
|
binary69 marked this conversation as resolved.
Outdated
|
||
| make_sequence_device(tmp_path, "device_0", start=1.0, end=8.0) | ||
| make_sequence_device(tmp_path, "device_1", start=0.0, end=10.0) | ||
|
|
||
| experiment = Experiment( | ||
| root_folder=tmp_path, | ||
| modality_config=make_modality_config("device_0", "device_1"), | ||
| ) | ||
|
|
||
| # Union: start = min(1.0, 0.0) = 0.0, end = max(8.0, 10.0) = 10.0 | ||
| assert experiment.start_time == pytest.approx( | ||
| 0.0 | ||
| ), f"Expected start_time=0.0, got {experiment.start_time}" | ||
| assert experiment.end_time == pytest.approx( | ||
| 10.0 | ||
| ), f"Expected end_time=10.0, got {experiment.end_time}" | ||
|
pollytur marked this conversation as resolved.
Outdated
|
||
|
|
||
|
binary69 marked this conversation as resolved.
|
||
|
|
||
| def test_experiment_single_device_time_range(tmp_path): | ||
| """With a single device, start_time and end_time should match that device's range.""" | ||
| make_sequence_device(tmp_path, "device_0", start=2.0, end=9.0) | ||
|
|
||
| experiment = Experiment( | ||
| root_folder=tmp_path, | ||
| modality_config=make_modality_config("device_0"), | ||
| ) | ||
|
|
||
| assert experiment.start_time == pytest.approx(2.0) | ||
| assert experiment.end_time == pytest.approx(9.0) | ||
|
|
||
|
|
||
| def test_experiment_start_end_time_three_devices(tmp_path): | ||
|
binary69 marked this conversation as resolved.
Outdated
|
||
| """With three devices, start_time and end_time should reflect the union of all three.""" | ||
| make_sequence_device(tmp_path, "device_0", start=0.0, end=10.0) | ||
| make_sequence_device(tmp_path, "device_1", start=1.0, end=8.0) | ||
| make_sequence_device(tmp_path, "device_2", start=2.0, end=9.0) | ||
|
|
||
| experiment = Experiment( | ||
| root_folder=tmp_path, | ||
| modality_config=make_modality_config("device_0", "device_1", "device_2"), | ||
| ) | ||
|
|
||
| # Union: start = min(0.0, 1.0, 2.0) = 0.0, end = max(10.0, 8.0, 9.0) = 10.0 | ||
| assert experiment.start_time == pytest.approx(0.0) | ||
| assert experiment.end_time == pytest.approx(10.0) | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.