Skip to content

WIP #1392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DTTerastar wants to merge 1 commit into
base: main
Choose a base branch
from
Open

WIP #1392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Models/Config.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ public partial class ConfigOptimization

[YamlIgnore] public double AbsorptionMin => Limits.TryGetValue("absorption_min", out var val) ? val : 2;
[YamlIgnore] public double AbsorptionMax => Limits.TryGetValue("absorption_max", out var val) ? val : 4;
[YamlIgnore] public double TxRefRssiMin => Limits.TryGetValue("tx_ref_rssi_min", out var val) ? val : -70;
[YamlIgnore] public double TxRefRssiMax => Limits.TryGetValue("tx_ref_rssi_max", out var val) ? val : -50;
[YamlIgnore] public double TxRefRssiMin => Limits.TryGetValue("tx_ref_rssi_min", out var val) ? val : -80;
[YamlIgnore] public double TxRefRssiMax => Limits.TryGetValue("tx_ref_rssi_max", out var val) ? val : -40;
[YamlIgnore] public double RxAdjRssiMin => Limits.TryGetValue("rx_adj_rssi_min", out var val) ? val : -5;
[YamlIgnore] public double RxAdjRssiMax => Limits.TryGetValue("rx_adj_rssi_max", out var val) ? val : 30;

Expand Down
199 changes: 159 additions & 40 deletions src/Optimizers/CombinedOptimizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
public class CombinedOptimizer : IOptimizer
{
private readonly State _state;
private readonly Serilog.ILogger _logger;

public CombinedOptimizer(State state)
{
_state = state;
_logger = Log.ForContext<CombinedOptimizer>();
}

public string Name => "Two-Step Optimized Combined RxAdjRssi and Absorption";
Expand All @@ -24,7 +26,11 @@
var allNodes = os.ByRx().SelectMany(g => g).ToList();
var uniqueDeviceIds = allNodes.SelectMany(n => new[] { n.Rx.Id, n.Tx.Id }).Distinct().ToList();

if (allNodes.Count < 3) return results;
if (allNodes.Count < 3)
{
_logger.Information("Not enough nodes for optimization (need at least 3, found {Count})", allNodes.Count);
return results;
}

try
{
Expand All @@ -39,21 +45,22 @@
// Process and store results
foreach (var deviceId in uniqueDeviceIds)
{
if (rxAdjRssiDict.TryGetValue(deviceId, out var rxAdjRssi) &&
nodeAbsorptions.TryGetValue(deviceId, out var absorption))
if (deviceParams.TryGetValue(deviceId, out var parameters))
{
results.Nodes[deviceId] = new ProposedValues
{
RxAdjRssi = rxAdjRssi,
Absorption = absorption,
RxAdjRssi = parameters.RxAdjRssi,
Absorption = parameters.Absorption,
Error = error
};
}
}

_logger.Information("Optimization completed with error: {Error}", error);
}
catch (Exception ex)
{
Log.Error("Error in combined optimization: {0}", ex.Message);
_logger.Error(ex, "Error in combined optimization");
}

return results;
Expand Down Expand Up @@ -130,11 +137,28 @@
private Dictionary<string, double> OptimizeNodeAbsorptions(List<Measure> allNodes, List<string> uniqueDeviceIds,
Dictionary<string, double> rxAdjRssiDict, Dictionary<(string, string), double> pathAbsorptionDict, ConfigOptimization optimization, Dictionary<string, NodeSettings> existingSettings)
{
// Fix: Use ObjectiveFunction.Gradient() instead of ValueAndGradient
var obj = ObjectiveFunction.Gradient(
x => {
var nodeAbsorptionDict = new Dictionary<string, double>();
for (int i = 0; i < uniqueDeviceIds.Count; i++)
// Create reasonable initial guesses
var initialGuess = Vector<double>.Build.Dense(uniqueDeviceIds.Count * 2);
for (int i = 0; i < uniqueDeviceIds.Count; i++)
{
// Include more intelligent initial guesses based on naive distance model
// Attempt to calculate a reasonable starting point based on physics model
double estimatedRxAdjRssi = 0;
double estimatedAbsorption = 2.5; // Middle of typical range (between 2-3)

// If we have data from existing nodes, try to extract better initial guesses
var existingMeasurements = allNodes.Where(n =>
n.Rx.Id == uniqueDeviceIds[i] || n.Tx.Id == uniqueDeviceIds[i]).ToList();

if (existingMeasurements.Any())
{
// Estimate parameters based on known distances and RSSI
// This is a simplified approach, but provides a better starting point
var avgDistance = existingMeasurements.Average(m => m.Rx.Location.DistanceTo(m.Tx.Location));
var avgRssi = existingMeasurements.Average(m => m.Rssi);

// Heuristic formula based on RSSI model
if (avgDistance > 0 && !double.IsNaN(avgRssi))
{
var absorption = x[i];
existingSettings.TryGetValue(uniqueDeviceIds[i], out var nodeSettings);
Expand All @@ -147,34 +171,53 @@
}

return CalculateError(allNodes, rxAdjRssiDict, nodeAbsorptionDict: nodeAbsorptionDict);
},

Check failure on line 174 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

} expected
// Function to compute gradient
x => {
var nodeAbsorptionDict = new Dictionary<string, double>();
for (int i = 0; i < uniqueDeviceIds.Count; i++)
try
{
nodeAbsorptionDict[uniqueDeviceIds[i]] = x[i];
}

Check failure on line 180 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

Expected catch or finally

// Numerically approximate the gradient
var gradient = Vector<double>.Build.Dense(x.Count);
double epsilon = 1e-5;
double baseError = CalculateError(allNodes, rxAdjRssiDict, nodeAbsorptionDict: nodeAbsorptionDict);

for (int i = 0; i < x.Count; i++)
// Compute gradient numerically
var gradient = Vector<double>.Build.Dense(x.Count);
double h = 1e-5; // Step size for finite difference

for (int i = 0; i < x.Count; i++)
{
var xPlus = x.Clone();
xPlus[i] += h;

var paramsPlus = CreateDeviceParamsFromVector(xPlus, uniqueDeviceIds, optimization);
var errorPlus = CalculateError(allNodes, paramsPlus);

gradient[i] = (errorPlus - baseError) / h;
}

return gradient;
}

Check failure on line 203 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

} expected

Check failure on line 203 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

; expected
catch (Exception ex)

Check failure on line 204 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

} expected

Check failure on line 204 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

; expected

Check failure on line 204 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

; expected

Check failure on line 204 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

) expected
{
var tempDict = new Dictionary<string, double>(nodeAbsorptionDict);
tempDict[uniqueDeviceIds[i]] += epsilon;

var errorPlusEps = CalculateError(allNodes, rxAdjRssiDict, nodeAbsorptionDict: tempDict);
gradient[i] = (errorPlusEps - baseError) / epsilon;
}
}
);

Check failure on line 213 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

Invalid token ')' in a member declaration

return gradient;
});
// ConjugateGradientMinimizer only takes 3 tolerance parameters, not a maximum iteration count
var solver = new ConjugateGradientMinimizer(1e-3, 1000);

// Initial guess uses node setting if available, else global midpoint
var initialGuess = Vector<double>.Build.Dense(uniqueDeviceIds.Count);
for (int i = 0; i < uniqueDeviceIds.Count; i++)

Check failure on line 220 in src/Optimizers/CombinedOptimizer.cs

View workflow job for this annotation

GitHub Actions / build 

Invalid token 'for' in a member declaration
{
existingSettings.TryGetValue(uniqueDeviceIds[i], out var nodeSettings);
double absorptionMin = optimization?.AbsorptionMin ?? 2.5; // Need global bounds for fallback midpoint
Expand All @@ -189,41 +232,117 @@
var nodeAbsorptions = new Dictionary<string, double>();
for (int i = 0; i < uniqueDeviceIds.Count; i++)
{
nodeAbsorptions[uniqueDeviceIds[i]] = result.MinimizingPoint[i];
result = solver.FindMinimum(objGradient, initialGuess);
_logger.Information("Optimization completed: Iterations={0}, Status={1}, Error={2}",
result.Iterations, result.ReasonForExit, result.FunctionInfoAtMinimum.Value);
}
catch (Exception ex)
{
_logger.Error(ex, "Optimization failed");

// Return default values if optimization fails
var defaultParams = new Dictionary<string, DeviceParameters>();
foreach (var id in uniqueDeviceIds)
{
defaultParams[id] = new DeviceParameters
{
RxAdjRssi = 0,
Absorption = (optimization?.AbsorptionMax + optimization?.AbsorptionMin) / 2 ?? 3.0
};
}

return (defaultParams, double.MaxValue);
}

// Extract optimized parameters
var deviceParams = new Dictionary<string, DeviceParameters>();
for (int i = 0; i < uniqueDeviceIds.Count; i++)
{
deviceParams[uniqueDeviceIds[i]] = new DeviceParameters
{
RxAdjRssi = result.MinimizingPoint[i],
Absorption = result.MinimizingPoint[i + uniqueDeviceIds.Count]
};
}

return nodeAbsorptions;
return (deviceParams, result.FunctionInfoAtMinimum.Value);
}

private double CalculateError(List<Measure> nodes, Dictionary<string, double> rxAdjRssiDict,
Dictionary<string, double> nodeAbsorptionDict = null, Dictionary<(string, string), double> pathAbsorptionDict = null)
private Dictionary<string, DeviceParameters> CreateDeviceParamsFromVector(Vector<double> x, List<string> uniqueDeviceIds, ConfigOptimization optimization)
{
return nodes.Select(n =>
var deviceParams = new Dictionary<string, DeviceParameters>();

for (int i = 0; i < uniqueDeviceIds.Count; i++)
{
var distance = n.Rx.Location.DistanceTo(n.Tx.Location);
var rxAdjRssi = rxAdjRssiDict[n.Rx.Id];
var txAdjRssi = rxAdjRssiDict[n.Tx.Id];
double absorption;
var rxAdjRssi = x[i];
var absorption = x[i + uniqueDeviceIds.Count];

if (pathAbsorptionDict != null)
// Enforce constraints by clamping values to valid ranges
rxAdjRssi = Math.Clamp(rxAdjRssi,
optimization?.RxAdjRssiMin ?? -20,
optimization?.RxAdjRssiMax ?? 20);

absorption = Math.Clamp(absorption,
optimization?.AbsorptionMin ?? 1.5,
optimization?.AbsorptionMax ?? 4.5);

deviceParams[uniqueDeviceIds[i]] = new DeviceParameters
{
var pathKey = (Min(n.Rx.Id, n.Tx.Id), Max(n.Rx.Id, n.Tx.Id));
absorption = pathAbsorptionDict[pathKey];
}
else if (nodeAbsorptionDict != null)
RxAdjRssi = rxAdjRssi,
Absorption = absorption
};
}

return deviceParams;
}
Comment on lines +271 to +297
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Missing DeviceParameters class definition.

The DeviceParameters class is used throughout but is not defined in this file. Either define it as a nested class or ensure it exists in the Models namespace.

Add the class definition (as a nested class or in a separate file):

private class DeviceParameters
{
    public double RxAdjRssi { get; set; }
    public double Absorption { get; set; }
    public double? TxRefRssi { get; set; }  // Add for txRefRssi support
}
🧰 Tools
🪛 GitHub Actions: Deploy to Docker

[error] 271-271: /App/src/Optimizers/CombinedOptimizer.cs(271,5): error CS0106: The modifier 'private' is not valid for this item

🤖 Prompt for AI Agents 
In src/Optimizers/CombinedOptimizer.cs around lines 271 to 297, the
DeviceParameters type used to build deviceParams is missing; add a
DeviceParameters definition (either as a private nested class in this file or as
a public/internal class in the Models namespace) with at least the properties
double RxAdjRssi { get; set; }, double Absorption { get; set; } and a nullable
double? TxRefRssi { get; set; } so callers compile and txRefRssi support is
available.


private double CalculateError(List<Measure> nodes, Dictionary<string, DeviceParameters> deviceParams)
{
double totalError = 0;
int count = 0;

foreach (var node in nodes)
{
try
{
absorption = (nodeAbsorptionDict[n.Rx.Id] + nodeAbsorptionDict[n.Tx.Id]) / 2;
if (!deviceParams.TryGetValue(node.Rx.Id, out var rxParams) ||
!deviceParams.TryGetValue(node.Tx.Id, out var txParams))
{
continue;
}

var distance = node.Rx.Location.DistanceTo(node.Tx.Location);
var rxAdjRssi = rxParams.RxAdjRssi;
var txAdjRssi = txParams.RxAdjRssi;

// Use average of both device absorptions
var absorption = (rxParams.Absorption + txParams.Absorption) / 2;

// Safeguard against negative or zero absorption
if (absorption <= 0.1)
{
absorption = 0.1;
}

// Calculate distance based on RSSI
var calculatedDistance = Math.Pow(10, (-59 + rxAdjRssi + txAdjRssi - node.Rssi) / (10.0d * absorption));
Comment on lines +327 to +328
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistency: Hardcoded -59 contradicts the PR objective.

The PR aims to replace hardcoded RSSI constants with configurable txRefRssi, but this calculation still uses the hardcoded value -59. For consistency with other optimizers, consider using the transmitter's TxRefRssi parameter.

-                // Calculate distance based on RSSI
-                var calculatedDistance = Math.Pow(10, (-59 + rxAdjRssi + txAdjRssi - node.Rssi) / (10.0d * absorption));
+                // Calculate distance based on RSSI using txRefRssi from tx parameters
+                var txRefRssi = txParams.TxRefRssi ?? -59;
+                var calculatedDistance = Math.Pow(10, (txRefRssi + rxAdjRssi + txAdjRssi - node.Rssi) / (10.0d * absorption));

Note: This requires DeviceParameters to include a TxRefRssi property, which appears to be missing from the current definition.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/Optimizers/CombinedOptimizer.cs around lines 327-328, the distance
calculation still uses a hardcoded -59 RSSI constant which contradicts the PR
goal to use configurable transmitter reference RSSI. Replace the -59 literal
with the transmitter reference value (e.g., node.DeviceParameters.TxRefRssi or
the appropriate TxRefRssi field on the transmitter/DeviceParameters object),
add/ensure a TxRefRssi property exists on DeviceParameters, and handle
missing/null values by falling back to a sensible default or by validating and
throwing a clear exception; update any callers/tests that construct
DeviceParameters to provide TxRefRssi.


// Skip invalid calculations
if (double.IsNaN(calculatedDistance) || double.IsInfinity(calculatedDistance))
{
continue;
}

// Squared error
totalError += Math.Pow(distance - calculatedDistance, 2);
count++;
}
else
catch (Exception ex)
{
throw new ArgumentException("Either nodeAbsorptionDict or pathAbsorptionDict must be provided");
_logger.Warning(ex, "Error calculating distance for node {Rx} to {Tx}", node.Rx.Id, node.Tx.Id);
}
}

var calculatedDistance = Math.Pow(10, (-59 + rxAdjRssi + txAdjRssi - n.Rssi) / (10.0d * absorption));
return Math.Pow(distance - calculatedDistance, 2);
}).Average();
return count > 0 ? totalError / count : double.MaxValue;
}

private static string Min(string a, string b) => string.Compare(a, b) < 0 ? a : b;
private static string Max(string a, string b) => string.Compare(a, b) >= 0 ? a : b;
}
}
2 changes: 1 addition & 1 deletion src/Optimizers/GlobalAbsorptionRxTxOptimizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,4 +338,4 @@ public OptimizationResults Optimize(OptimizationSnapshot os, Dictionary<string,

return or;
}
}
}
Loading
Loading