-
Notifications
You must be signed in to change notification settings - Fork 5
Description
Location
package/src/pyaslreport/modalities/asl/processor.py:264-266
Description
_convert_units_to_milliseconds unconditionally multiplies time fields by 1000. If a BIDS JSON was generated manually or from a scanner that outputs values in milliseconds (e.g., PostLabelingDelay: 1800), it silently double-converts them to 1,800,000 without any heuristic safeguards.
Reproduction (on main branch)
Script
from pyaslreport.modalities.asl.processor import ASLProcessor
proc = object.__new__(ASLProcessor)
session = {"PostLabelingDelay": 1800, "EchoTime": 15}
proc._convert_units_to_milliseconds(session)
# Result: PLD=1800000, EchoTime=15000 (both double-converted)Processor / API Normalized Response (main branch)
Input payload with values already in milliseconds:
{
"PostLabelingDelay": 1800,
"EchoTime": 15,
"RepetitionTime": 4000.0,
"MagneticFieldStrength": 3.0,
"Manufacturer": "Generic"
}Output after processor normalization (bug present):
{
"PostLabelingDelay": 1800000,
"EchoTime": 15000,
"RepetitionTimePreparation": 4000000.0,
"MagneticFieldStrength": 3.0,
"Manufacturer": "Generic"
}Automated Results on main branch
| # | Test | Status | Detail |
|---|---|---|---|
| 1 | Seconds input (1.8s -> 1800ms) | PASS | |
| 2 | Already-ms input (1800 stays 1800) | FAIL | PLD=1800000, ET=15000 |
| 3 | List already-ms ([1800,2000,2200] stays same) | FAIL | [1800000, 2000000, 2200000] |
| 4 | List seconds input ([1.8,2.0,2.2] -> [1800,2000,2200]) | PASS |
Summary: PASSED=2 FAILED=2
Suggested Fix
Add a heuristic guard: if a scalar time value is > 10, or any element in a list is > 10, it is assumed to already be in milliseconds and the conversion is skipped. This threshold is safe because typical ASL time values in seconds are all < 10 (EchoTime ~0.01-0.03s, PLD ~1-3s, RepetitionTime ~3-6s).
I will wait for your response on this one. If you think this solution is not viable then lets come up with a better solution. Then, I implement it.