-
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 1 commit
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,163 @@ | ||
| import shutil | ||
| from contextlib import contextmanager | ||
| from pathlib import Path | ||
|
|
||
| import numpy as np | ||
|
binary69 marked this conversation as resolved.
|
||
| import pytest | ||
| import yaml | ||
|
|
||
| from experanto.experiment import Experiment | ||
|
|
||
| EXPERIMENT_ROOT = Path("tests/experiment_data") | ||
|
|
||
|
|
||
| @contextmanager | ||
| def create_two_device_experiment( | ||
| device0_start=0.0, | ||
| device0_end=10.0, | ||
| device1_start=1.0, | ||
| device1_end=8.0, | ||
| sampling_rate=10.0, | ||
| ): | ||
| """Create a temporary experiment with two sequence devices with different time ranges.""" | ||
| try: | ||
| for device_name, start, end in [ | ||
| ("device_0", device0_start, device0_end), | ||
| ("device_1", device1_start, device1_end), | ||
| ]: | ||
| device_root = EXPERIMENT_ROOT / device_name | ||
| (device_root / "meta").mkdir(parents=True, exist_ok=True) | ||
|
|
||
| n_samples = int((end - start) * sampling_rate) + 1 | ||
| timestamps = np.linspace(start, end, n_samples) | ||
| data = np.random.rand(n_samples, 5) | ||
|
|
||
| np.save(device_root / "timestamps.npy", timestamps) | ||
| np.save(device_root / "data.npy", data) | ||
|
|
||
|
binary69 marked this conversation as resolved.
|
||
| 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) | ||
|
|
||
| yield EXPERIMENT_ROOT | ||
|
|
||
| finally: | ||
| shutil.rmtree(EXPERIMENT_ROOT) | ||
|
|
||
|
binary69 marked this conversation as resolved.
Outdated
|
||
|
|
||
| @contextmanager | ||
| def create_three_device_experiment(): | ||
| """Create a temporary experiment with three sequence devices with different time ranges.""" | ||
| try: | ||
| for device_name, start, end in [ | ||
| ("device_0", 0.0, 10.0), | ||
| ("device_1", 1.0, 8.0), | ||
| ("device_2", 2.0, 9.0), | ||
| ]: | ||
| device_root = EXPERIMENT_ROOT / device_name | ||
| (device_root / "meta").mkdir(parents=True, exist_ok=True) | ||
|
|
||
| sampling_rate = 10.0 | ||
| n_samples = int((end - start) * sampling_rate) + 1 | ||
| timestamps = np.linspace(start, end, n_samples) | ||
| data = np.random.rand(n_samples, 5) | ||
|
|
||
| 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) | ||
|
|
||
| yield EXPERIMENT_ROOT | ||
|
|
||
| finally: | ||
| shutil.rmtree(EXPERIMENT_ROOT) | ||
|
|
||
|
|
||
| def get_two_device_config(): | ||
| return { | ||
| "device_0": {"interpolation": {"sampling_rate": 10.0, "offset": 0.0}}, | ||
| "device_1": {"interpolation": {"sampling_rate": 10.0, "offset": 0.0}}, | ||
| } | ||
|
|
||
|
|
||
| def test_experiment_start_end_time_reflects_union(): | ||
| """ | ||
| Experiment.start_time and end_time should reflect the union | ||
| of all device time ranges — the earliest start and latest end | ||
| across all devices, not just the last loaded device. | ||
| """ | ||
| with create_two_device_experiment( | ||
| device0_start=1.0, | ||
| device0_end=8.0, | ||
| device1_start=0.0, | ||
| device1_end=10.0, | ||
| ) as experiment_path: | ||
| experiment = Experiment( | ||
| root_folder=experiment_path, | ||
| modality_config=get_two_device_config(), | ||
| ) | ||
|
|
||
| # 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}" | ||
|
|
||
|
|
||
| def test_experiment_single_device_time_range(): | ||
| """With a single device, start_time and end_time should match that device's range.""" | ||
| with create_two_device_experiment( | ||
| device0_start=2.0, | ||
| device0_end=9.0, | ||
| ) as experiment_path: | ||
| config = {"device_0": {"interpolation": {"sampling_rate": 10.0, "offset": 0.0}}} | ||
| experiment = Experiment( | ||
| root_folder=experiment_path, | ||
| modality_config=config, | ||
| ) | ||
|
|
||
| assert experiment.start_time == pytest.approx(2.0) | ||
| assert experiment.end_time == pytest.approx(9.0) | ||
|
|
||
|
|
||
| def test_experiment_start_end_time_three_devices(): | ||
| """With three devices, start_time and end_time should reflect the union of all three — | ||
| earliest start and latest end across all devices.""" | ||
| with create_three_device_experiment() as experiment_path: | ||
| config = { | ||
| "device_0": {"interpolation": {"sampling_rate": 10.0, "offset": 0.0}}, | ||
| "device_1": {"interpolation": {"sampling_rate": 10.0, "offset": 0.0}}, | ||
| "device_2": {"interpolation": {"sampling_rate": 10.0, "offset": 0.0}}, | ||
| } | ||
| experiment = Experiment( | ||
| root_folder=experiment_path, | ||
| modality_config=config, | ||
| ) | ||
|
|
||
| # 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.