Found auditing pointer discipline, per the standing rule that CheckPointer
comes before every dereference.
THE LEAKS. CNeuronLSTM::feedForward allocated forget_gate, input_gate,
output_gate and new_content on the heap and deleted them only on the success
path. Eight error returns sit between the first allocation and that delete,
and every one of them abandoned whatever had been built so far. calcHidden-
Gradients was the same shape with fourteen returns past MemoryGradient. This
is the CPU path, which is the only path this machine has - no OpenCL, no
DirectML - so it ran on every era of every LSTM and CONVLSTM member.
Fixed by construction rather than by adding deletes: none of the five buffers
escapes its function, so each is now an automatic object. The return itself
destroys them, which means the leak cannot come back the next time someone
adds an error path - which is exactly how it got here.
CalculateGate had to change shape for that: it now fills a caller-supplied
CArrayDouble and answers bool, instead of handing back an object each caller
was responsible for deleting on its own error paths and none of them did. It
also allocated BEFORE testing `gate`, leaking on that very check, and never
tested `sequence` at all before dereferencing it. Both arguments are checked
first now. Protected virtual with three call sites, all in this file - no
public API moves.
THE UNCHECKED DEREFERENCES. The input-gradient loop did four rounds of
`temp = SomeGate.At(i); con = temp.getConnections().At(n); value +=
temp.getGradient() * con.weight` with no check on either pointer, and At()
answers NULL for an out-of-range index rather than failing loudly. The four
copies are now one AccumulateGateInputGradient() that checks the layer, the
neuron and the connection. The line above them read `temp.getConnections()`
off whatever the previous loop happened to leave in `temp` - NULL if
OutputLayer was empty - and is now checked too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MetaEditor: "declaration of 'eta' hides global variable" (Math.mqh:792
vs Network.mqh:80). The standard library's Math\Stat\Math.mqh declares a
local `double eta` in its incomplete-gamma branch, and our bare global
of the same name is in scope there.
Same fault as the b1/b2/lr/momentum macros retired in ea2552e: a
single-token global name living in a header that library code gets
compiled beside. The library cannot move, so ours does - 112 references
across 13 files, whole-word only.
Named g_eta rather than g_learningRate to stay inside the vocabulary
already around it (ETA_DECAY_FACTOR, ETA_MIN, m_etaCeiling, etaBefore),
all of which are untouched and none of which shadow anything.
One log line said "continuing to explore without decaying eta", where
the word was prose rather than a symbol reference; that reads "the
learning rate" now instead of naming a variable at the trader.
Scanned for the next occurrence rather than waiting for it: the only
other bare lowercase globals in the tree are eaName and tableschema,
both distinctive enough not to collide with a library local.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gate's NormalUpperTail was a hand-rolled Abramowitz & Stegun 26.2.17
approximation. Its own comment gave the reason - "drags a chain of headers
behind it" - and that turned out to be one file: Math\Stat\Normal.mqh
includes only Math.mqh, which includes nothing. Swapped for Cody's rational
approximation in the library (~18 significant digits vs |error| < 7.5e-8).
No past verdict changes: at the z the gate operates on, the difference is
orders of magnitude below DEPLOY_FAMILY_WISE_ALPHA.
Adopting it needed the four bare macros in AI\Network.mqh gone first.
"#define b1 AdamBeta1" collides with an identifier in Math.mqh, so the
include would have macro-expanded the library's own local and failed to
compile - the same landmine that made the original author rename the
approximation's coefficients to ntB1..ntB5 rather than use the reference's
b1..b5. lr, b2 and momentum are the same class of hazard: single-token
global macros in a 52k-line codebase. All four now resolve to the input
names they always aliased, which is a pure textual identity - verified zero
bare occurrences remain.
Also:
- SelectionSort over the buffered signals was O(n^2) with an O(n^2) count of
StructToTime calls, because the comparison rebuilt both datetimes from the
six int date fields every time. Now materialises the keys once and does an
insertion sort; ArraySort cannot permute a struct array. IsEarlier goes
with it, MakeDateTime becomes SignalTime.
- Seven FileOpen sites lacked FILE_SHARE_READ|FILE_SHARE_WRITE, including
AtomicWriteBegin, which stages every model save. All 43 sites now carry
them - an exclusive open fails outright when another process holds the
path, which here has meant a silently skipped save.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Moves CLayer neuron construction to AI/Impl/Layer.mqh to keep Network.mqh clean
- Unifies four previously duplicated architecture initialisation blocks (MLP/CONV/LSTM/HYBRID) into a single shared function
- Eliminates risk of behavioural drift where one architecture missed a setter, causing mismatched feature sets or targets