Skip to content

Fix backpropagation math and thread-safety in neural network core - #2

Draft
maybepritz with Copilot wants to merge 3 commits into
masterfrom
copilot/fix-backpropagation-weights-update
Draft

Fix backpropagation math and thread-safety in neural network core#2
maybepritz with Copilot wants to merge 3 commits into
masterfrom
copilot/fix-backpropagation-weights-update

Conversation

Copilot AI commented Feb 1, 2026

Copy link
Copy Markdown

Three critical mathematical errors in backpropagation were causing incorrect gradient flow:

Backpropagation Issues

Weight update ordering - Weights were updated before computing error for previous layer, propagating incorrect gradients:

// Before: Used already-updated weights ❌
weights = optimizer.update(weights, gradient, id);
return weights.transpose().multiply(grad);

// After: Compute error first, then update ✅
Matrix errorForPrevLayer = weights.transpose().multiply(grad);
weights = optimizer.update(weights, gradient, id);
return errorForPrevLayer;

Activation derivatives - Derivatives computed from post-activation values instead of pre-activation:

  • Added preActivation field to store z before activation
  • Changed getGradient() to use preActivation.map(activation::derivative) instead of output.map(...)
  • Fixed Sigmoid.derivative() to compute sigmoid(x) from raw input first

Dropout edge case - Added check for dropoutRate < 1.0 to prevent division by zero

Concurrency

Replaced HashMap with ConcurrentHashMap in AdamOptimizer for thread-safe multi-threaded training.

Performance

Use MatrixFactory.create(rows, cols) instead of weights.copy().scale(0) for zero matrix initialization in Adam optimizer state.

Original prompt

Описание проблем

В коде библиотеки JNeuro обнаружены следующие критические ошибки и проблемы, которые необходимо исправить:


🔴 Критические ошибки

1. Backprop использует уже обновлённые веса (DenseLayer.java)

Файл: src/main/java/io/github/maybepritz/layers/DenseLayer.java

Проблема: В методе backpropagate() сначала обновляются веса, а затем используются для вычисления ошибки предыдущего слоя. Это неправильно — нужно сначала вычислить ошибку, потом обновить веса.

Текущий код (строки 89-98):

weights = config.getOptimizer().update(weights, weightGradient, layerId);

if (useBias) {
    bias = config.getOptimizer().update(bias, grad, layerId + "_bias");
}

return weights.transpose().multiply(grad);  // ❌ Использует уже ОБНОВЛЁННЫЕ веса!

Исправление:

// Сначала вычисляем ошибку для предыдущего слоя (ДО обновления весов!)
Matrix errorForPrevLayer = weights.transpose().multiply(grad);

// Теперь обновляем веса
weights = config.getOptimizer().update(weights, weightGradient, layerId);

if (useBias) {
    bias = config.getOptimizer().update(bias, grad, layerId + "_bias");
}

return errorForPrevLayer;

2. Производная считается от output вместо pre-activation (DenseLayer.java)

Проблема: В методе getGradient() производная функции активации вычисляется от output (уже активированных значений), но для Tanh и ReLU производная должна вычисляться от pre-activation значения z.

Текущий forward():

@Override
public Matrix forward(Matrix input) {
    this.input = input;
    Matrix z = weights.multiply(input);

    if (useBias) {
        z.addColumn(bias);
    }

    this.output = z.map(activation::activate);
    // z не сохраняется!

Исправление: Добавить поле private Matrix preActivation; и сохранять z:

private Matrix preActivation;  // Добавить поле

@Override
public Matrix forward(Matrix input) {
    this.input = input;
    Matrix z = weights.multiply(input);

    if (useBias) {
        z.addColumn(bias);
    }

    this.preActivation = z;  // Сохраняем для backprop
    this.output = z.map(activation::activate);
    // ... остальной код

И изменить getGradient():

@Override
public Matrix getGradient(Matrix error, NetworkConfig config) {
    // Используем preActivation вместо output для производной
    Matrix grad = preActivation.map(activation::derivative);
    grad = grad.elementMultiply(error);
    // ... остальной код

3. Ошибка в производной Sigmoid (Sigmoid.java)

Файл: src/main/java/io/github/maybepritz/activations/Sigmoid.java

Проблема: После исправления п.2, derivative будет получать исходное x, а не sigmoid(x). Нужно исправить формулу.

Текущий код:

@Override
public double derivative(double x){
    return x * (1.0 - x);  // Это для случая когда x = sigmoid(z)
}

Исправление:

@Override
public double derivative(double x){
    double sig = activate(x);  // Вычисляем sigmoid от исходного x
    return sig * (1.0 - sig);
}

🟠 Серьёзные проблемы

4. Деление на ноль при dropout (DenseLayer.java)

Проблема: Если dropoutRate = 1.0, произойдёт деление на ноль.

Текущий код:

if (training && dropoutRate > 0) {
    dropoutMask = Matrix.randomMask(output.getRows(), output.getCols(), 1 - dropoutRate);
    output = output.elementMultiply(dropoutMask);
    output = output.scale(1.0 / (1 - dropoutRate));  // ❌ Деление на 0 если dropoutRate = 1
}

Исправление:

if (training && dropoutRate > 0 && dropoutRate < 1.0) {
    dropoutMask = Matrix.randomMask(output.getRows(), output.getCols(), 1 - dropoutRate);
    output = output.elementMultiply(dropoutMask);
    output = output.scale(1.0 / (1 - dropoutRate));
}

5. Thread-safety в AdamOptimizer (AdamOptimizer.java)

Файл: src/main/java/io/github/maybepritz/optimizers/AdamOptimizer.java

Проблема: Использование HashMap небезопасно при многопоточном обучении.

Текущий код:

private final Map<String, Matrix> m = new HashMap<>();
private final Map<String, Matrix> v = new HashMap<>();
private final Map<String, Integer> t = new HashMap<>();

Исправление: Заменить на ConcurrentHashMap:

import java.util.concurrent.ConcurrentHashMap;

private final Map<String, Matrix> m = new ConcurrentHashMap<>();
private final Map<String, Matrix> v = new ConcurrentHashMap<>();
private final Map<String, Integer> t = new ConcurrentHashMap<>();

6. Неэффективная инициализация в AdamOptimizer (AdamOptimizer.java)

Проблема: Создание копии матрицы и умножение на 0 неэффективно.

Текущий код:

if (!m.containsKey(layerId)) {
    m.put(layerId, weights.copy().scale(0));
    v.put(layerId, weights.copy().scale(0));

Исправление: Использовать MatrixFactory для создания нулевой матрицы:

import io.github.maybepritz.utils.MatrixFactory;

if (!m.containsKey(layerId)) {
    m.put(laye...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

Copilot AI and others added 2 commits February 1, 2026 15:40
Co-authored-by: maybepritz <79636615+maybepritz@users.noreply.github.com>
Co-authored-by: maybepritz <79636615+maybepritz@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix backpropagation weight update issue in DenseLayer Fix backpropagation math and thread-safety in neural network core Feb 1, 2026
Copilot AI requested a review from maybepritz February 1, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants