refactor(units): route all conversions through UnitPolicy.convert
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
* 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
|
* units using the host UnitPolicy. Logs a warning when the per-pressure
|
||||||
* 3x relative to the previous pressure level — that almost always means the
|
* median y jumps by more than 3x relative to the previous pressure level —
|
||||||
* curve file is corrupt (mixed units, swapped rows) and the predict module
|
* that almost always means the curve file is corrupt (mixed units, swapped
|
||||||
* would otherwise silently produce nonsense values.
|
* 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 = {};
|
const normalized = {};
|
||||||
let prevMedianY = null;
|
let prevMedianY = null;
|
||||||
|
|
||||||
for (const [pressureKey, pair] of Object.entries(section || {})) {
|
for (const [pressureKey, pair] of Object.entries(section || {})) {
|
||||||
const canonicalPressure = convertUnitValue(
|
const canonicalPressure = unitPolicy.convert(
|
||||||
Number(pressureKey),
|
Number(pressureKey),
|
||||||
fromPressureUnit,
|
fromPressureUnit,
|
||||||
toPressureUnit,
|
toPressureUnit,
|
||||||
`${sectionName} pressure axis`
|
`${sectionName} pressure axis`,
|
||||||
);
|
);
|
||||||
const xArray = Array.isArray(pair?.x) ? pair.x.map(Number) : [];
|
const xArray = Array.isArray(pair?.x) ? pair.x.map(Number) : [];
|
||||||
const yArray = Array.isArray(pair?.y)
|
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) {
|
if (!xArray.length || !yArray.length || xArray.length !== yArray.length) {
|
||||||
throw new Error(`Invalid ${sectionName} section at pressure '${pressureKey}'.`);
|
throw new Error(`Invalid ${sectionName} section at pressure '${pressureKey}'.`);
|
||||||
@@ -74,21 +59,23 @@ function normalizeMachineCurve(rawCurve, unitPolicy, logger) {
|
|||||||
return {
|
return {
|
||||||
nq: normalizeCurveSection(
|
nq: normalizeCurveSection(
|
||||||
rawCurve.nq,
|
rawCurve.nq,
|
||||||
|
unitPolicy,
|
||||||
curveUnits.flow,
|
curveUnits.flow,
|
||||||
canonicalFlow,
|
canonicalFlow,
|
||||||
curveUnits.pressure,
|
curveUnits.pressure,
|
||||||
canonicalPressure,
|
canonicalPressure,
|
||||||
'nq',
|
'nq',
|
||||||
logger
|
logger,
|
||||||
),
|
),
|
||||||
np: normalizeCurveSection(
|
np: normalizeCurveSection(
|
||||||
rawCurve.np,
|
rawCurve.np,
|
||||||
|
unitPolicy,
|
||||||
curveUnits.power,
|
curveUnits.power,
|
||||||
canonicalPower,
|
canonicalPower,
|
||||||
curveUnits.pressure,
|
curveUnits.pressure,
|
||||||
canonicalPressure,
|
canonicalPressure,
|
||||||
'np',
|
'np',
|
||||||
logger
|
logger,
|
||||||
),
|
),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -114,4 +101,4 @@ function readCanonical(unitPolicy, type) {
|
|||||||
return (unitPolicy.canonical || {})[type] || null;
|
return (unitPolicy.canonical || {})[type] || null;
|
||||||
}
|
}
|
||||||
|
|
||||||
module.exports = { normalizeMachineCurve, normalizeCurveSection, convertUnitValue };
|
module.exports = { normalizeMachineCurve, normalizeCurveSection };
|
||||||
|
|||||||
@@ -79,6 +79,12 @@ function buildQHCurve(predictors, ctrlPct, options = {}) {
|
|||||||
if (!pf.inputCurve || typeof pf.inputCurve !== 'object') {
|
if (!pf.inputCurve || typeof pf.inputCurve !== 'object') {
|
||||||
return { error: NO_CURVE_ERROR, points: [] };
|
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 x = Number.isFinite(+ctrlPct) ? +ctrlPct : (pf.currentX ?? 0);
|
||||||
const RHO = 999.1; // kg/m³ — water at ~15 °C
|
const RHO = 999.1; // kg/m³ — water at ~15 °C
|
||||||
const G = 9.80665; // m/s²
|
const G = 9.80665; // m/s²
|
||||||
@@ -103,7 +109,8 @@ function buildQHCurve(predictors, ctrlPct, options = {}) {
|
|||||||
for (const p of pressures) {
|
for (const p of pressures) {
|
||||||
pf.fDimension = p;
|
pf.fDimension = p;
|
||||||
const QM3s = pf.y(x);
|
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 {
|
} finally {
|
||||||
pf.fDimension = originalF;
|
pf.fDimension = originalF;
|
||||||
|
|||||||
@@ -50,7 +50,7 @@ class FlowController {
|
|||||||
return await host.executeSequence(parameter);
|
return await host.executeSequence(parameter);
|
||||||
|
|
||||||
case 'flowmovement': {
|
case 'flowmovement': {
|
||||||
const canonicalFlowSetpoint = host._convertUnitValue(
|
const canonicalFlowSetpoint = host.unitPolicy.convert(
|
||||||
parameter,
|
parameter,
|
||||||
host.unitPolicy.output.flow,
|
host.unitPolicy.output.flow,
|
||||||
host.unitPolicy.canonical.flow,
|
host.unitPolicy.canonical.flow,
|
||||||
|
|||||||
@@ -247,12 +247,6 @@ class Machine extends BaseDomain {
|
|||||||
if (!this.isUnitValidForType(type, u)) throw new Error(`Unsupported unit '${u}' for ${type} measurement.`);
|
if (!this.isUnitValidForType(type, u)) throw new Error(`Unsupported unit '${u}' for ${type} measurement.`);
|
||||||
return u;
|
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'; }
|
_measurementPositionForMetric(metricId) { return metricId === 'power' ? 'atEquipment' : 'downstream'; }
|
||||||
_resolveProcessRangeForMetric(metricId, predicted, measured) {
|
_resolveProcessRangeForMetric(metricId, predicted, measured) {
|
||||||
let processMin = NaN; let processMax = NaN;
|
let processMin = NaN; let processMax = NaN;
|
||||||
|
|||||||
@@ -5,7 +5,6 @@ const { UnitPolicy } = require('generalFunctions');
|
|||||||
const {
|
const {
|
||||||
normalizeMachineCurve,
|
normalizeMachineCurve,
|
||||||
normalizeCurveSection,
|
normalizeCurveSection,
|
||||||
convertUnitValue,
|
|
||||||
} = require('../../src/curves/curveNormalizer');
|
} = require('../../src/curves/curveNormalizer');
|
||||||
|
|
||||||
function makePolicy() {
|
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', () => {
|
test('normalizeCurveSection: warns on cross-pressure median > 3x jump', () => {
|
||||||
|
const policy = makePolicy();
|
||||||
const logger = captureLogger();
|
const logger = captureLogger();
|
||||||
const section = {
|
const section = {
|
||||||
1000: { x: [0, 50, 100], y: [0, 5, 10] }, // median 5
|
1000: { x: [0, 50, 100], y: [0, 5, 10] }, // median 5
|
||||||
1100: { x: [0, 50, 100], y: [0, 50, 100] }, // median 50 (10x jump)
|
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));
|
const hit = logger.warns.find((w) => /Curve anomaly/.test(w));
|
||||||
assert.ok(hit, `expected a Curve anomaly warning, got: ${JSON.stringify(logger.warns)}`);
|
assert.ok(hit, `expected a Curve anomaly warning, got: ${JSON.stringify(logger.warns)}`);
|
||||||
assert.match(hit, /pressure 1100/);
|
assert.match(hit, /pressure 1100/);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('normalizeCurveSection: does not warn on smooth progressions', () => {
|
test('normalizeCurveSection: does not warn on smooth progressions', () => {
|
||||||
|
const policy = makePolicy();
|
||||||
const logger = captureLogger();
|
const logger = captureLogger();
|
||||||
const section = {
|
const section = {
|
||||||
1000: { x: [0, 50, 100], y: [0, 5, 10] },
|
1000: { x: [0, 50, 100], y: [0, 5, 10] },
|
||||||
1100: { x: [0, 50, 100], y: [0, 6, 11] },
|
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);
|
assert.equal(logger.warns.filter((w) => /Curve anomaly/.test(w)).length, 0);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('normalizeCurveSection: throws when x/y length mismatch', () => {
|
test('normalizeCurveSection: throws when x/y length mismatch', () => {
|
||||||
|
const policy = makePolicy();
|
||||||
assert.throws(
|
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/
|
/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/);
|
|
||||||
});
|
|
||||||
|
|||||||
@@ -27,6 +27,10 @@ function makeHost({
|
|||||||
unitPolicy: {
|
unitPolicy: {
|
||||||
canonical: { flow: 'm3/s' },
|
canonical: { flow: 'm3/s' },
|
||||||
output: { flow: 'm3/h' },
|
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),
|
isValidActionForMode: (action) => allowedActions.has(action),
|
||||||
isValidSourceForMode: () => allowedSources,
|
isValidSourceForMode: () => allowedSources,
|
||||||
@@ -38,10 +42,6 @@ function makeHost({
|
|||||||
return { moved: sp };
|
return { moved: sp };
|
||||||
},
|
},
|
||||||
calcCtrl: (canonicalFlow) => { host.calls.calcCtrl.push(canonicalFlow); return canonicalFlow / 2; },
|
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;
|
return host;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user