Implementing alignment based optimization for single-cell data by scjlee · Pull Request #72 · artyomovlab/dualsimplex

scjlee · 2026-07-31T20:01:49Z

Implement new optimization algorithm for single-cell data. src/optimization_dsa
- This algorithm add an additional alignment term to the loss function of positivity optimization.
Use match.arg function check if user input method is valid in optim_config. R/opimization
Implement new logger system. src/optimization_logger
- The new error logger records errors as a long format data.frame. This data.frame contains 3 columns: {iteration, metric_name, value}. This design increase the freedom to add/remove tracked values
- For backward compatibility, the logger class contains a to_legacy_matrix method for converting data.frame to the old 21 column matrix .
- Modify optimize_solution to support output of the history data.frame.
- The long format could make plotting interface cleaner:
- Currently, only optimize_alignment use this new logger. All other algorithms are kept the same.
Implement new error calculator. src/optimization_error_metrics.
- User can define there own error terms as a IErrorCalculator class.
- Calculators can be registered to a single CompositeErrorCalculator object which provide calculate method to compute all the registered error. This design is aim to reduce boilerplate code.
- Implement 4 basic error calculators: DeconvErrorCalculator, HingeProportionsErrorCalculator, HingeBasisErrorCalculator, ScaleNormErrorCalculator.
- Both hinge error calculator perform a single W (or H) reconstruction for both beta_error and neg_basis (or lambda_error and neg_props), which reduce total computation load when matrix is big.
- Currently, only optimize_alignment use this new error calculator.. All other algorithms are kept the same.
Implement GitHub action for automatic RcppExports and documents generation. .github/workflows
- Each time when a new commit is pushed, run Rcpp::compileAttributes and roxygen2::roxygenise to make sure all the R-Cpp bindings and documentations are uptodate. If there is any change, GitHub action will automatically create a new commit with modified files.
- Theoretically, it is same as running devtools::document. Once syncing is passed, package should be ready to install via remotes::install_github.

denklewer · 2026-07-31T20:21:40Z

 #' @export
 optim_config <- function(
-  method = "positivity", # positivity/coordinate_descent/theta
+  method = c("positivity", "coordinate_descent", "theta", "alignment"), # let R handel selection and checking


handel is a typo

denklewer · 2026-07-31T20:26:02Z

@@ -1,2 +1,3 @@
 PKG_LIBS = $(LAPACK_LIBS) $(BLAS_LIBS) $(FLIBS)
 PKG_CPPFLAGS = -DARMA_64BIT_WORD=1
+CXX_STD = CXX17


GitHub action use C++20 but the current R which failed in CMD INSTALL step. Is most like to due to issue with RcppSpdlog.

Currently (2026/08/10) R use C++17 as the default compiler since version 4.4, and C++20 become default after 4.6 ‘where available’. I think it is save to use C++17 as our compiler because its is available (but not necessarily installed) on all known R platforms. according to CRAN.

ok, let's keep it but can you please create an issue to support other versions. I think once we change logging as we discussed it should work

denklewer · 2026-07-31T20:26:50Z

+
+
+
+//' Main training loop with all gradient steps. This is the main algorithm for now. 


add documentation please. looks like this one copied

denklewer · 2026-07-31T20:57:16Z

+#include <map>
+#include <fstream>
+
+class ILogger {


shouldn't we have something like errortracker

Popular framework like pytorch and tensorflow use logger for error tracking. I would like to keep this convention and don't see too much issue still. But I defiantly open to discuss alternative naming.

And I don't see it will be confused with spdlog which is usually called as spdl or spdlog.

I understand this but if a new guy will debug error he met. He will try to search "logger" first and will be transferred to the class which is not related to logger

Sorry, I don't understand why logger is closely related to debugging Could you provide some examples?

I meet error in DEBUG or ERROR log. I open vscode and search "log" or "logger" to find the places where error logging happened, to investigate the error. But will find this class, which does not do this logging. So I will spend some time trying to figure it out.

denklewer · 2026-08-03T21:49:27Z

+    }
+};
+
+class CsvLogger : public ILogger {


probably not useful for us

…provide freedom of tuning

…optimization

…add legacy matrix conversion support

…ted header and source files

…ew results

… iteration loop to start from 1

…lper functions in the optimization loop

…cal formulas

…kward compatibility

…gradient descent (PGD)

…lgorithm.

scjlee · 2026-08-10T16:05:27Z

      D_h = Dh
    ))
  },
+  legacy_random_invertible = function(proj, kwargs = NULL) {


This is the "quasi-old" method I used to test the original alignment optimization which failing with the newest initialization. Since the new PGD-based optimization works with new initialization so far. I can remove it if you like.

we agreed to remove this

scjlee

Implement new alignment optimization algorithm using projected gradient descent. It should fix the convergence problem encountered for the previous inverse-based optimization
Implement get_history and get_history_long function for accessing the optimization history. They only work with histories created by the new logger.
Explaining implementation choice to most of the previous comments.

denklewer · 2026-08-12T20:33:55Z


+    #' @description
+    #' Get history of optimization.
+    get_history = function() {


history$history sounds ambiguous. It can be confused with points history.
I think it should be error_log or metrics_log if you like, since it is what it is.
Also I think method should be called differently

denklewer · 2026-08-12T20:36:21Z

+    virtual void log_metadata(const std::map<std::string, std::string>& metadata) = 0;
+};
+
+class RcppLogger : public ILogger {


RcppLogger still looks like you are logging some Rcpp events, since this is only error tracking, I guess name should be transparent. Also I think the name should be directly related to the R naming, to improve code understanding (I mean we should avoid transforming entity RcppLogger into history$history

I agree. Rename it to ErrorLogger or others will be better.

Meanwhile, I don't understand what do you mean "we should avoid transforming entity RcppLogger into history$history". Are you against to use logger system?

No, I just mean that it should be transparent everywhere. both RcppLogger and history$history are to vague.
So I vote just for very similar names both in R and C++ for easy navigation.

denklewer · 2026-08-12T20:41:20Z

+#include <map>
+#include <fstream>
+
+class ILogger {


I understand this but if a new guy will debug error he met. He will try to search "logger" first and will be transferred to the class which is not related to logger

denklewer · 2026-08-12T20:44:59Z

+}
+
+std::vector<Metric> HingeProportionsErrorCalculator::calculate(const OptimizationState& state) const {
+    arma::mat H = state.X * state.R;


shouldn't we make it H_inf just in case?

denklewer · 2026-08-12T20:48:28Z

+class ILogger {
+public:
+    virtual ~ILogger() = default;
+    virtual void log_metrics(int iteration, const std::vector<Metric>& metrics) = 0;


The name Ilogger does not give us clue of what this interface doing. Because if this is logger then it is expected that you will implement "log" method

I don't understand. Doesn't this interface require its child classes have both log_metrics and log_metadata implementations?

This is just naming issue for me. (As most of my comments, sorry!)
ILogger --> assumes we want log() method to be implemented.
for example IMetricsLogger -> means we want log_metrics() to be implemented.

Imagine you will have 10 implemented interfaces. It will be good if the name will be very transparent

scjlee

Can you resolve comments which you think is done? It is hard to track now.

denklewer · 2026-08-21T18:26:16Z

      D_h = Dh
    ))
  },
+  legacy_random_invertible = function(proj, kwargs = NULL) {


we agreed to remove this

denklewer · 2026-08-21T18:27:11Z

@@ -1,2 +1,3 @@
 PKG_LIBS = $(LAPACK_LIBS) $(BLAS_LIBS) $(FLIBS)
 PKG_CPPFLAGS = -DARMA_64BIT_WORD=1
+CXX_STD = CXX17


ok, let's keep it but can you please create an issue to support other versions. I think once we change logging as we discussed it should work

denklewer · 2026-08-21T21:19:15Z

+    virtual void log_metadata(const std::map<std::string, std::string>& metadata) = 0;
+};
+
+class RcppLogger : public ILogger {


No, I just mean that it should be transparent everywhere. both RcppLogger and history$history are to vague.
So I vote just for very similar names both in R and C++ for easy navigation.

denklewer · 2026-08-21T21:22:18Z

+class ILogger {
+public:
+    virtual ~ILogger() = default;
+    virtual void log_metrics(int iteration, const std::vector<Metric>& metrics) = 0;


This is just naming issue for me. (As most of my comments, sorry!)
ILogger --> assumes we want log() method to be implemented.
for example IMetricsLogger -> means we want log_metrics() to be implemented.

Imagine you will have 10 implemented interfaces. It will be good if the name will be very transparent

denklewer · 2026-08-21T21:27:15Z

+#include <map>
+#include <fstream>
+
+class ILogger {


I meet error in DEBUG or ERROR log. I open vscode and search "log" or "logger" to find the places where error logging happened, to investigate the error. But will find this class, which does not do this logging. So I will spend some time trying to figure it out.

denklewer · 2026-08-21T21:43:30Z

+        // Optimization Control Flow & Learning Rate Scheduling
+        // -------------------------------------------------------------
+        // TODO(cjlee): maybe we should compare to best_error_value instead?
+        double relative_change = (prev_error_value - current_error_value) / (std::abs(prev_error_value) + epsilon);


don't we have this implemented in other optimizations using these ?

stop_criteria_window = 800,
convergence_tol = 1e-9,

…se k=2

scjlee requested a review from denklewer July 31, 2026 20:01

denklewer requested changes Aug 3, 2026

View reviewed changes

scjlee force-pushed the dsa_optimization branch 2 times, most recently from 7ab323c to 21639e3 Compare August 7, 2026 00:27

scjlee and others added 26 commits August 10, 2026 10:18

Implement additional regularization with alignment theorem

12c4a27

Implement GitHub Action workflow for Rcpp/roxygen sync

c767588

chore: auto-update RcppExports and documentation [skip ci]

4339b2a

feat: adding coef_hinge_H argument back to optimize_alignment to …

fd9840f

…provide freedom of tuning

bug-fix: recover coef_hinge_H in optimize_solution for alignment …

6d844fc

…optimization

chore: auto-update RcppExports and documentation [skip ci]

c58992a

bug-fix: remove coefficient re-weighting

4ebcc08

feat: remove not used regularization and refactor optimize_alignment

6663875

chore: auto-update RcppExports and documentation [skip ci]

8e13a27

doc: save implementation plan for refactoring error logging

dc96880

doc: add 5th milestone: compatibility layer

7cd758a

feat: implement modular error calculation and logging system

16b7ebd

feat: expose new long format optimization log to DualSimplexSolver

7179fff

doc: refine implementation plan for the new logging mechanism

866d77f

feat: single-pass hinge loss calculation

54bdcf8

refactor: transition error tracking to logger-based architecture and …

995b173

…add legacy matrix conversion support

refactor: decouple optimization metrics and logging logic into dedica…

61a98b4

…ted header and source files

feat: implement AlignmentErrorCalculator for optimize_alignment

4f5df8f

fix: align iteration numbers in optimization history when appending n…

bd0ab56

…ew results

refactor: implement logging for initial optimization state and update…

735a224

… iteration loop to start from 1

refactor: simplify metric extraction by using ensemble metrics and he…

cd36c0f

…lper functions in the optimization loop

chore: auto-update RcppExports and documentation [skip ci]

a51e250

feat: add legacy_random_invertible for testing.

499c7db

refactor: rename variable names for clariy and in sync with mathemati…

8e0781d

…cal formulas

chore: auto-update RcppExports and documentation [skip ci]

61ce8b8

feat: Prioritize error plotting to use new long-form history with bac…

6ce3f1b

…kward compatibility

scjlee and others added 8 commits August 10, 2026 10:25

refactor: remove legacy logging in matrix from optimize_alignment

f46d2e5

doc: manually update documentation and RcppExport [skip ci]

4e495d3

fix: proper from_idx calculation in optimize_solution [skip ci]

9492e63

feat: implement new alignment optimization algorithm using progected …

291bc01

…gradient descent (PGD)

chore: auto-update RcppExports and documentation [skip ci]

6ae01c7

fix: add alignment_pgd to the list of optimization method

e2f442e

chore: auto-update RcppExports and documentation [skip ci]

605e23d

feat: remove inverse-base alignment optimization in favor PGD-based a…

1b1028f

…lgorithm.

scjlee force-pushed the dsa_optimization branch from 8589001 to 1b1028f Compare August 10, 2026 15:29

chore: auto-update RcppExports and documentation [skip ci]

8cb9afd

scjlee commented Aug 10, 2026

View reviewed changes

scjlee added 2 commits August 10, 2026 11:45

fix: remove unused CSVLogger

14988ba

feat: implement getters for optimization history from the logger

1fa16e4

scjlee force-pushed the dsa_optimization branch from e2a10e9 to 1fa16e4 Compare August 10, 2026 17:20

chore: auto-update RcppExports and documentation [skip ci]

9ba36d0

scjlee commented Aug 10, 2026

View reviewed changes

scjlee requested a review from denklewer August 10, 2026 19:05

scjlee and others added 4 commits August 11, 2026 11:31

feat: implement "reduce LR on plateau" strategy in optimize_alignment

164600e

chore: auto-update RcppExports and documentation [skip ci]

2229f0a

feat: track the 'effective learning rate' used in backtracking loop

278d13f

fix: calculating true gradients without any shortcut

1f55d05

scjlee force-pushed the dsa_optimization branch from a996fca to 1f55d05 Compare August 12, 2026 23:05

scjlee and others added 2 commits August 13, 2026 11:13

refactor: clean-up unused memory allocation and parameters

87ae497

chore: auto-update RcppExports and documentation [skip ci]

f360683

denklewer requested changes Aug 21, 2026

View reviewed changes

scjlee commented Aug 21, 2026

View reviewed changes

denklewer requested changes Aug 21, 2026

View reviewed changes

fix: prevent matrix dimension drop in optimize_solution for edge ca…

a3024d0

…se k=2




		//' Main training loop with all gradient steps. This is the main algorithm for now.

Uh oh!

Conversation

scjlee commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scjlee Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scjlee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scjlee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

scjlee commented Jul 31, 2026 •

edited

Loading

scjlee Aug 10, 2026 •

edited

Loading