From 455f15dc553164e1b313a5259aa96d8f83ceaef8 Mon Sep 17 00:00:00 2001 From: znetsixe Date: Sat, 23 May 2026 13:43:26 +0200 Subject: [PATCH] refactor(units): route all conversions through UnitPolicy.convert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delete the legacy _convertUnitValue helper on the domain and the duplicate convertUnitValue export on curveNormalizer; both were identical to UnitPolicy.convert. Callers in flowController, the curve normalizer, and buildQHCurve now go through this.unitPolicy. The contract in .claude/refactor/CONTRACTS.md §6 named these as the target migration; this finishes the rollout for rotatingMachine. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/curves/curveNormalizer.js | 39 ++++++++---------------- src/display/workingCurves.js | 9 +++++- src/flow/flowController.js | 2 +- src/specificClass.js | 6 ---- test/basic/curveNormalizer.basic.test.js | 19 ++++-------- test/basic/flowController.basic.test.js | 8 ++--- 6 files changed, 32 insertions(+), 51 deletions(-) diff --git a/src/curves/curveNormalizer.js b/src/curves/curveNormalizer.js index e5715a6..42b3061 100644 --- a/src/curves/curveNormalizer.js +++ b/src/curves/curveNormalizer.js @@ -1,39 +1,24 @@ -const { convert } = require('generalFunctions'); - -/** - * Strict numeric unit conversion. Mirrors specificClass._convertUnitValue - * so the curve normalizer is testable without a Machine instance. - */ -function convertUnitValue(value, fromUnit, toUnit, contextLabel = 'unit conversion') { - const numeric = Number(value); - if (!Number.isFinite(numeric)) { - throw new Error(`${contextLabel}: value '${value}' is not finite`); - } - if (!fromUnit || !toUnit || fromUnit === toUnit) return numeric; - return convert(numeric).from(fromUnit).to(toUnit); -} - /** * Convert one curve section (nq or np) from supplied units to canonical - * units. Logs a warning when the per-pressure median y jumps by more than - * 3x relative to the previous pressure level — that almost always means the - * curve file is corrupt (mixed units, swapped rows) and the predict module - * would otherwise silently produce nonsense values. + * units using the host UnitPolicy. Logs a warning when the per-pressure + * median y jumps by more than 3x relative to the previous pressure level — + * that almost always means the curve file is corrupt (mixed units, swapped + * rows) and the predict module would otherwise silently produce nonsense. */ -function normalizeCurveSection(section, fromYUnit, toYUnit, fromPressureUnit, toPressureUnit, sectionName, logger) { +function normalizeCurveSection(section, unitPolicy, fromYUnit, toYUnit, fromPressureUnit, toPressureUnit, sectionName, logger) { const normalized = {}; let prevMedianY = null; for (const [pressureKey, pair] of Object.entries(section || {})) { - const canonicalPressure = convertUnitValue( + const canonicalPressure = unitPolicy.convert( Number(pressureKey), fromPressureUnit, toPressureUnit, - `${sectionName} pressure axis` + `${sectionName} pressure axis`, ); const xArray = Array.isArray(pair?.x) ? pair.x.map(Number) : []; const yArray = Array.isArray(pair?.y) - ? pair.y.map((v) => convertUnitValue(v, fromYUnit, toYUnit, `${sectionName} output`)) + ? pair.y.map((v) => unitPolicy.convert(v, fromYUnit, toYUnit, `${sectionName} output`)) : []; if (!xArray.length || !yArray.length || xArray.length !== yArray.length) { throw new Error(`Invalid ${sectionName} section at pressure '${pressureKey}'.`); @@ -74,21 +59,23 @@ function normalizeMachineCurve(rawCurve, unitPolicy, logger) { return { nq: normalizeCurveSection( rawCurve.nq, + unitPolicy, curveUnits.flow, canonicalFlow, curveUnits.pressure, canonicalPressure, 'nq', - logger + logger, ), np: normalizeCurveSection( rawCurve.np, + unitPolicy, curveUnits.power, canonicalPower, curveUnits.pressure, canonicalPressure, 'np', - logger + logger, ), }; } @@ -114,4 +101,4 @@ function readCanonical(unitPolicy, type) { return (unitPolicy.canonical || {})[type] || null; } -module.exports = { normalizeMachineCurve, normalizeCurveSection, convertUnitValue }; +module.exports = { normalizeMachineCurve, normalizeCurveSection }; diff --git a/src/display/workingCurves.js b/src/display/workingCurves.js index d4631bf..738d530 100644 --- a/src/display/workingCurves.js +++ b/src/display/workingCurves.js @@ -79,6 +79,12 @@ function buildQHCurve(predictors, ctrlPct, options = {}) { if (!pf.inputCurve || typeof pf.inputCurve !== 'object') { return { error: NO_CURVE_ERROR, points: [] }; } + const policy = options.unitPolicy || predictors.unitPolicy; + if (!policy) { + return { error: 'No unitPolicy available for Q-axis conversion', points: [] }; + } + const flowFrom = policy.canonical?.flow || policy.canonical?.('flow'); + const flowTo = policy.output?.flow || policy.output?.('flow'); const x = Number.isFinite(+ctrlPct) ? +ctrlPct : (pf.currentX ?? 0); const RHO = 999.1; // kg/m³ — water at ~15 °C const G = 9.80665; // m/s² @@ -103,7 +109,8 @@ function buildQHCurve(predictors, ctrlPct, options = {}) { for (const p of pressures) { pf.fDimension = p; const QM3s = pf.y(x); - points.push({ Q: QM3s * 3600, H: p / (RHO * G), dpPa: p }); + const Q = policy.convert(QM3s, flowFrom, flowTo, 'buildQHCurve Q-axis'); + points.push({ Q, H: p / (RHO * G), dpPa: p }); } } finally { pf.fDimension = originalF; diff --git a/src/flow/flowController.js b/src/flow/flowController.js index 48d890d..fc3d286 100644 --- a/src/flow/flowController.js +++ b/src/flow/flowController.js @@ -50,7 +50,7 @@ class FlowController { return await host.executeSequence(parameter); case 'flowmovement': { - const canonicalFlowSetpoint = host._convertUnitValue( + const canonicalFlowSetpoint = host.unitPolicy.convert( parameter, host.unitPolicy.output.flow, host.unitPolicy.canonical.flow, diff --git a/src/specificClass.js b/src/specificClass.js index 0790122..17d0744 100644 --- a/src/specificClass.js +++ b/src/specificClass.js @@ -247,12 +247,6 @@ class Machine extends BaseDomain { if (!this.isUnitValidForType(type, u)) throw new Error(`Unsupported unit '${u}' for ${type} measurement.`); return u; } - _convertUnitValue(value, from, to, ctx = 'unit conversion') { - const n = Number(value); - if (!Number.isFinite(n)) throw new Error(`${ctx}: value '${value}' is not finite`); - if (!from || !to || from === to) return n; - return convert(n).from(from).to(to); - } _measurementPositionForMetric(metricId) { return metricId === 'power' ? 'atEquipment' : 'downstream'; } _resolveProcessRangeForMetric(metricId, predicted, measured) { let processMin = NaN; let processMax = NaN; diff --git a/test/basic/curveNormalizer.basic.test.js b/test/basic/curveNormalizer.basic.test.js index 6f50d4c..36531c6 100644 --- a/test/basic/curveNormalizer.basic.test.js +++ b/test/basic/curveNormalizer.basic.test.js @@ -5,7 +5,6 @@ const { UnitPolicy } = require('generalFunctions'); const { normalizeMachineCurve, normalizeCurveSection, - convertUnitValue, } = require('../../src/curves/curveNormalizer'); function makePolicy() { @@ -50,39 +49,33 @@ test('normalizeMachineCurve: converts pressure mbar -> Pa and flow m3/h -> m3/s' }); test('normalizeCurveSection: warns on cross-pressure median > 3x jump', () => { + const policy = makePolicy(); const logger = captureLogger(); const section = { 1000: { x: [0, 50, 100], y: [0, 5, 10] }, // median 5 1100: { x: [0, 50, 100], y: [0, 50, 100] }, // median 50 (10x jump) }; - normalizeCurveSection(section, 'm3/h', 'm3/h', 'mbar', 'mbar', 'nq', logger); + normalizeCurveSection(section, policy, 'm3/h', 'm3/h', 'mbar', 'mbar', 'nq', logger); const hit = logger.warns.find((w) => /Curve anomaly/.test(w)); assert.ok(hit, `expected a Curve anomaly warning, got: ${JSON.stringify(logger.warns)}`); assert.match(hit, /pressure 1100/); }); test('normalizeCurveSection: does not warn on smooth progressions', () => { + const policy = makePolicy(); const logger = captureLogger(); const section = { 1000: { x: [0, 50, 100], y: [0, 5, 10] }, 1100: { x: [0, 50, 100], y: [0, 6, 11] }, }; - normalizeCurveSection(section, 'm3/h', 'm3/h', 'mbar', 'mbar', 'nq', logger); + normalizeCurveSection(section, policy, 'm3/h', 'm3/h', 'mbar', 'mbar', 'nq', logger); assert.equal(logger.warns.filter((w) => /Curve anomaly/.test(w)).length, 0); }); test('normalizeCurveSection: throws when x/y length mismatch', () => { + const policy = makePolicy(); assert.throws( - () => normalizeCurveSection({ 1000: { x: [0, 50], y: [0, 5, 10] } }, 'm3/h', 'm3/s', 'mbar', 'Pa', 'nq', null), + () => normalizeCurveSection({ 1000: { x: [0, 50], y: [0, 5, 10] } }, policy, 'm3/h', 'm3/s', 'mbar', 'Pa', 'nq', null), /Invalid nq section/ ); }); - -test('convertUnitValue: identity when units match or missing', () => { - assert.equal(convertUnitValue(42, 'm3/h', 'm3/h'), 42); - assert.equal(convertUnitValue(42, null, null), 42); -}); - -test('convertUnitValue: throws on non-finite input', () => { - assert.throws(() => convertUnitValue('not-a-number', 'm3/h', 'm3/s', 'test'), /not finite/); -}); diff --git a/test/basic/flowController.basic.test.js b/test/basic/flowController.basic.test.js index d8881f3..662ccfc 100644 --- a/test/basic/flowController.basic.test.js +++ b/test/basic/flowController.basic.test.js @@ -27,6 +27,10 @@ function makeHost({ unitPolicy: { canonical: { flow: 'm3/s' }, output: { flow: 'm3/h' }, + convert: (val, from, to, label) => { + host.calls.convertUnit.push({ val, from, to, label }); + return val * 1000; // pretend m3/h -> m3/s factor + }, }, isValidActionForMode: (action) => allowedActions.has(action), isValidSourceForMode: () => allowedSources, @@ -38,10 +42,6 @@ function makeHost({ return { moved: sp }; }, calcCtrl: (canonicalFlow) => { host.calls.calcCtrl.push(canonicalFlow); return canonicalFlow / 2; }, - _convertUnitValue: (val, from, to, label) => { - host.calls.convertUnit.push({ val, from, to, label }); - return val * 1000; // pretend m3/h -> m3/s factor - }, }; return host; }