Skip to content

preserve input star columns during particle export #433

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

Closed
shahpnmlab wants to merge 16 commits into from
+69 −6
Closed

preserve input star columns during particle export #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2068f79
test n_tilts=1 export
shahpnmlab Apr 17, 2025
0614f93
cont
shahpnmlab Apr 17, 2025
becc4d9
maybe?
shahpnmlab Apr 17, 2025
31e0981
hmmm
shahpnmlab Apr 17, 2025
188c709
test again
shahpnmlab Apr 17, 2025
464eb5e
Merge branch 'warpem:main' into main
shahpnmlab Apr 18, 2025
5daf02a
Fix particle-series export when n_tilt=1
shahpnmlab Apr 18, 2025
6c51516
Merge branch 'warpem:main' into main
shahpnmlab Jun 24, 2025
e19a6f4
Merge branch 'warpem:main' into main
shahpnmlab Jul 24, 2025
14784f0
Merge branch 'warpem:main' into main
shahpnmlab Oct 29, 2025
fe284ab
preserve column data from input_star -1st attempt
shahpnmlab Oct 29, 2025
a899a0b
added the necessary using directive to WarpWorker/WarpWorker.cs.
shahpnmlab Oct 29, 2025
ad096b2
uhhh?
shahpnmlab Oct 29, 2025
8ace542
maybe?
shahpnmlab Oct 29, 2025
6c25a35
has it generalised?
shahpnmlab Oct 29, 2025
e96199a
Increase MaxRequestBodySize to 500 MB
shahpnmlab Nov 6, 2025
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
5 changes: 3 additions & 2 deletions WarpLib/WorkerWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,15 +463,16 @@ public void TomoExportParticleSubtomos(string path, ProcessingOptionsTomoSubReco
angles));
}

public void TomoExportParticleSeries(string path, ProcessingOptionsTomoSubReconstruction options, float3[] coordinates, float3[] angles, string pathsRelativeTo, string pathTableOut)
public void TomoExportParticleSeries(string path, ProcessingOptionsTomoSubReconstruction options, float3[] coordinates, float3[] angles, string pathsRelativeTo, string pathTableOut, Dictionary<string, string[]> additionalColumns)
{
SendCommand(new NamedSerializableObject("TomoExportParticleSeries",
path,
options,
coordinates,
angles,
pathsRelativeTo,
pathTableOut));
pathTableOut,
additionalColumns));
Comment on lines +466 to +475
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep the original public overload.

Replacing Warp.WorkerWrapper.TomoExportParticleSeries with a 7-parameter signature is a source/binary break for existing consumers compiled against the old API. Since WarpWorker.WarpWorkerProcess.EvaluateCommand already treats the extra payload item as optional, keep the old overload and forward it to the new one instead of replacing it.

💡 Suggested compatibility overload 
+        public void TomoExportParticleSeries(string path, ProcessingOptionsTomoSubReconstruction options, float3[] coordinates, float3[] angles, string pathsRelativeTo, string pathTableOut)
+        {
+            TomoExportParticleSeries(path, options, coordinates, angles, pathsRelativeTo, pathTableOut, null);
+        }
+
         public void TomoExportParticleSeries(string path, ProcessingOptionsTomoSubReconstruction options, float3[] coordinates, float3[] angles, string pathsRelativeTo, string pathTableOut, Dictionary<string, string[]> additionalColumns)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void TomoExportParticleSeries(string path, ProcessingOptionsTomoSubReconstruction options, float3[] coordinates, float3[] angles, string pathsRelativeTo, string pathTableOut, Dictionary<string, string[]> additionalColumns)
{
SendCommand(new NamedSerializableObject("TomoExportParticleSeries",
path,
options,
coordinates,
angles,
pathsRelativeTo,
pathTableOut));
pathTableOut,
additionalColumns));
public void TomoExportParticleSeries(string path, ProcessingOptionsTomoSubReconstruction options, float3[] coordinates, float3[] angles, string pathsRelativeTo, string pathTableOut)
{
TomoExportParticleSeries(path, options, coordinates, angles, pathsRelativeTo, pathTableOut, null);
}
public void TomoExportParticleSeries(string path, ProcessingOptionsTomoSubReconstruction options, float3[] coordinates, float3[] angles, string pathsRelativeTo, string pathTableOut, Dictionary<string, string[]> additionalColumns)
{
SendCommand(new NamedSerializableObject("TomoExportParticleSeries",
path,
options,
coordinates,
angles,
pathsRelativeTo,
pathTableOut,
additionalColumns));
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@WarpLib/WorkerWrapper.cs` around lines 466 - 475, Public API was changed by
replacing the original TomoExportParticleSeries overload; restore the original
public overload (the previous signature without the additionalColumns parameter)
and have it forward to the new TomoExportParticleSeries method by calling it
with a null or empty Dictionary for additionalColumns. Update references to the
overload in the same class (WorkerWrapper.TomoExportParticleSeries) so the
original signature exists and simply delegates to the new implementation that
calls SendCommand with NamedSerializableObject; do not remove the new
7-parameter method.

}

public void MPAPrepareSpecies(string path, string stagingSave)
Expand Down
54 changes: 51 additions & 3 deletions WarpTools/Commands/Tiltseries/ExportParticlesTiltseries.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ class ExportParticlesTiltseriesOptions : DistributedOptions

class ExportParticlesTiltseries : BaseCommand
{
private Dictionary<string, string[]> _additionalColumns;

public override async Task Run(object options)
{
await base.Run(options);
Expand Down Expand Up @@ -167,6 +169,26 @@ public override async Task Run(object options)
}

ValidateInputStar(inputStar);
_additionalColumns = new Dictionary<string, string[]>();
var knownColumns = new HashSet<string>
{
"rlnCoordinateX", "rlnCoordinateY", "rlnCoordinateZ",
"rlnMicrographName", "rlnTomoName",
"rlnOriginX", "rlnOriginXAngst",
"rlnOriginY", "rlnOriginYAngst",
"rlnOriginZ", "rlnOriginZAngst",
"rlnPixelSize", "rlnImagePixelSize",
"rlnAngleRot", "rlnAngleTilt", "rlnAnglePsi",
"rlnOpticsGroup", "rlnOpticsGroupName"
};
foreach (var columnName in inputStar.GetColumnNames())
{
if (!knownColumns.Contains(columnName))
{
_additionalColumns.Add(columnName, inputStar.GetColumn(columnName));
}
}
Comment on lines +172 to +190
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The reserved-column filter is incomplete.

WarpTools.Commands.ExportParticlesTiltseries.ConstructSubvolumeOutputTable already generates rlnImageName, rlnCtfImage, rlnMagnification, rlnDetectorPixelSize, rlnCtfMaxResolution, rlnVoltage, and rlnSphericalAberration, but none of those are excluded here. Re-exporting a STAR that already contains any of them will collide with the downstream AddColumn calls in both the 3D path and WarpWorker.WarpWorkerProcess.EvaluateCommand, so you either fail on duplicate names or copy stale metadata back over the freshly generated values. Please reserve every exporter-owned output column here, or skip columns that already exist before adding them.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@WarpTools/Commands/Tiltseries/ExportParticlesTiltseries.cs` around lines 172
- 190, The filter that builds _additionalColumns from inputStar.GetColumnNames()
is missing several exporter-owned output names and so can re-add columns created
by ConstructSubvolumeOutputTable (e.g. rlnImageName, rlnCtfImage,
rlnMagnification, rlnDetectorPixelSize, rlnCtfMaxResolution, rlnVoltage,
rlnSphericalAberration), causing duplicate AddColumn failures later (see
WarpWorker.WarpWorkerProcess.EvaluateCommand). Update the knownColumns set in
ExportParticlesTiltseries.ConstructSubvolumeOutputTable to include all
exporter-owned output column names listed above (and any others
ConstructSubvolumeOutputTable generates), or alternatively change the loop that
populates _additionalColumns to skip any column that already exists in the
output table before adding (check existence via the same inputStar APIs) so
AddColumn is never called for reserved names.


string[] tiltSeriesIDs = inputStar.HasColumn("rlnMicrographName") ? inputStar.GetColumn("rlnMicrographName") : inputStar.GetColumn("rlnTomoName");
Dictionary<string, List<int>> tiltSeriesIdToParticleIndices =
GroupParticles(tiltSeriesIDs);
Expand Down Expand Up @@ -236,6 +258,21 @@ public override async Task Run(object options)
float3[] tsParticleXyzAngstroms = new float3[tsParticleIdx.Count];
float3[] tsParticleRotTiltPsi = new float3[tsParticleIdx.Count];

Dictionary<string, string[]> tsAdditionalColumns = new Dictionary<string, string[]>();
if (_additionalColumns.Count > 0)
{
foreach (var col in _additionalColumns)
{
var colData = new string[tsParticleIdx.Count];
for (int i = 0; i < tsParticleIdx.Count; i++)
{
colData[i] = col.Value[tsParticleIdx[i]];
}
tsAdditionalColumns.Add(col.Key, colData);
}
}


if (Helper.IsDebug)
Console.WriteLine($"{tsParticleIdx.Count} particles for {tiltSeries.Name}");

Expand Down Expand Up @@ -279,7 +316,8 @@ public override async Task Run(object options)
inputHasEulerAngles: inputHasEulerAngles,
outputPixelSize: cli.OutputPixelSize,
relativeToParticleStarFile: cli.OutputPathsRelativeToStarFile,
particleStarFile: cli.OutputStarFile);
particleStarFile: cli.OutputStarFile,
additionalColumns: tsAdditionalColumns);
lock (OutputStarTables)
OutputStarTables.Add(tiltSeries.Name, TiltSeriesTable);
}
Expand All @@ -302,7 +340,8 @@ public override async Task Run(object options)
pathTableOut: TempTiltSeriesParticleStarPath,
pathsRelativeTo: cli.OutputPathsRelativeToStarFile ?
Path.GetFullPath(OutputStarPath) :
Directory.GetCurrentDirectory());
Directory.GetCurrentDirectory(),
additionalColumns: tsAdditionalColumns);

// generate necessary metadata for particles.star
if (Helper.IsDebug)
Expand Down Expand Up @@ -697,7 +736,8 @@ private Star ConstructSubvolumeOutputTable(
bool inputHasEulerAngles,
float outputPixelSize,
bool relativeToParticleStarFile, // default is relative to working directory
string? particleStarFile
string? particleStarFile,
Dictionary<string, string[]> additionalColumns
)
{
int nParticles = xyz.Length;
Expand Down Expand Up @@ -807,6 +847,14 @@ private Star ConstructSubvolumeOutputTable(
table.RemoveColumn("rlnAnglePsi");
}

if (additionalColumns != null)
{
foreach (var pair in additionalColumns)
{
table.AddColumn(pair.Key, pair.Value);
}
}

return table;
}

Expand Down
16 changes: 15 additions & 1 deletion WarpWorker/WarpWorker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.Extensions.Hosting;
using System;
using System.Diagnostics;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
Expand Down Expand Up @@ -75,7 +76,7 @@ static async Task Main(string[] args)
webBuilder.UseKestrel(options =>
{
options.ListenAnyIP(Port);
options.Limits.MaxRequestBodySize = 104857600; // 100 MB
options.Limits.MaxRequestBodySize = 500 * 1024 * 1024; // 100 MB
})
Comment on lines 76 to 80
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 20, 2026

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

This hardcoded body-size bump just moves the failure point.

With Warp.WorkerWrapper.SendCommand now serializing Dictionary<string, string[]> payloads, request size scales with particle count and preserved-column count. Raising MaxRequestBodySize to 500 MB means large exports still fail once they cross the cap, while both client and worker must still materialize the full JSON in memory. A path/subset handoff would be much more robust here, or at minimum this limit should be configurable.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@WarpWorker/WarpWorker.cs` around lines 76 - 80, The hardcoded
MaxRequestBodySize bump in the web host (inside the UseKestrel options where
Port and options.Limits.MaxRequestBodySize are set) must be replaced with a
configurable approach and/or a streaming/path-handoff strategy to avoid forcing
large JSON payloads (see Warp.WorkerWrapper.SendCommand which now serializes
Dictionary<string,string[]>). Change the Kestrel configuration to read the limit
from configuration or environment (e.g., an AppSettings key like
"MaxRequestBodySizeBytes" with a sensible default) and wire that value into
options.Limits.MaxRequestBodySize rather than 500*1024*1024; additionally,
consider adding or wiring a flag/config ("UsePathHandoff" or
"EnableStreamingUploads") and update SendCommand callers to support a
path/subset handoff or streamed upload to eliminate full in-memory JSON if
enabled. Ensure references to Port and the configuration key are clear so
reviewers can locate and test the change.

.UseStartup<RESTStartup>()
.ConfigureLogging(logging => logging.SetMinimumLevel(LogLevel.Warning));
Expand Down Expand Up @@ -858,6 +859,19 @@ public static void EvaluateCommand(NamedSerializableObject Command)

TiltSeries T = new TiltSeries(Path);
T.ReconstructParticleSeries(Options, Coordinates, Angles, PathsRelativeTo, out TableOut);

Dictionary<string, string[]> AdditionalColumns = null;
if (Command.Content.Length > 6)
AdditionalColumns = (Dictionary<string, string[]>)Command.Content[6];

if (AdditionalColumns != null)
{
foreach (var col in AdditionalColumns)
{
TableOut.AddColumn(col.Key, col.Value);
}
}

T.SaveMeta();

if (!string.IsNullOrEmpty(PathTableOut))
Expand Down