BiorthBasis::Cylinder deprojection improvements by The9Cat · Pull Request #208 · EXP-code/EXP

The9Cat · 2026-03-13T16:26:55Z

Movitation

Needed a less cuspy option than exponential for attempting to produce a doughnut-shaped basis. So this adds a Toomre model for that purpose.

Improvements

I noticed that the numerical Abel transform had poor systematics in the inner profile. This is not a bug fix, but it is a noticeable improvement. The original version would not cause failure but could result in weaker EOF convergece.
I added all three numerical variants as source configuration choices and switched to the internal derivative form by default. I provided some verification and test routines for this in utils/Tests. These methods could be made user selectable but I have not done this in this PR.

…the Toomre model in the updated list of valid types

…ivative form, which is more accurate near the center

Copilot

Pull request overview

Adds a Toomre disk option for cylindrical-basis deprojection/conditioning, expands the Abel inversion implementations used during deprojection, and introduces several standalone test/verification executables under utils/Test.

Changes:

Add a new Toomre disk model and wire a configurable deprojection model selector (dmodel) into Cylindrical basis initialization.
Update EmpCylSL::create_deprojection to support multiple Abel inversion variants (Derivative/Subtraction/IBP) with derivative as default.
Add Deprojector/EmpDeproj test utilities and build targets in utils/Test.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
utils/Test/testEmpDeproj.cc	New CLI test comparing `EmpDeproj` vs `Deprojector` across profiles/methods
utils/Test/testDeproject.cc	New example program for `Deprojector` (sampled + analytic construction)
utils/Test/testDeprojPlummer.cc	New simple deprojection verification driver (currently hardwired)
utils/Test/EmpDeproj.cc	New empirical deprojection implementation used by test programs
utils/Test/EmpDeproj.H	Header for `EmpDeproj` test-class API
utils/Test/Deprojector.cc	New numerical Abel deprojection implementation and grid integration logic
utils/Test/Deprojector.H	Header/API for `Deprojector`
utils/Test/CubicSpline.cc	New spline helper for sampled-data deprojection path
utils/Test/CubicSpline.H	Header for `CubicSpline`
utils/Test/CMakeLists.txt	Adds new test executables to the build
include/EmpCylSL.H	Adds `AbelType` + `abel_type` member to control deprojection variant
include/DiskModels.H	Adds `Toomre` disk density model for deprojection conditioning
exputil/EmpCylSL.cc	Switchable Abel inversion variants used in deprojection pipeline
expui/BiorthBasis.cc	Adds `dmodel` config key + lookup map and uses it when `deproject` is enabled
expui/BiorthBasis.H	Introduces `DeprojType` enum + lookup map for deprojection disk models

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

include/DiskModels.H

expui/BiorthBasis.cc

utils/Test/testDeprojPlummer.cc

utils/Test/testEmpDeproj.cc

utils/Test/EmpDeproj.cc

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…Deproj for method variables

Copilot

Pull request overview

Adds a Toomre-based option for cylindrical deprojection and refines the numerical Abel inversion machinery, along with several small test/verification executables under utils/Test.

Changes:

Add a new Toomre disk model for deprojection and wire it into cylindrical basis initialization via a dmodel lookup.
Introduce multiple Abel inversion variants (derivative/subtraction/IBP) in the EmpCylSL deprojection path (defaulting to derivative).
Add standalone utils/Test deprojection utilities (Deprojector, EmpDeproj) and test drivers, and hook them into utils/Test/CMakeLists.txt.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
utils/Test/testEmpDeproj.cc	New CLI test comparing `EmpDeproj` against `Deprojector` for several analytic profiles.
utils/Test/testEmp.cc	New CLI test for donut-shaped (inner cut) empirical deprojection.
utils/Test/testDeproject.cc	New minimal examples for `Deprojector` from sampled data vs functor.
utils/Test/testDeprojPlummer.cc	New Plummer/Gaussian/Toomre `Deprojector` validation driver.
utils/Test/EmpDeproj.cc	Adds an empirical deprojection implementation used by the new test drivers.
utils/Test/EmpDeproj.H	Declares the `EmpDeproj` helper class and AbelType selection.
utils/Test/Deprojector.cc	Adds a standalone deprojection implementation with tail-matching and improved inner integration handling.
utils/Test/Deprojector.H	Declares the standalone `Deprojector` API used by tests.
utils/Test/CubicSpline.cc	Adds a basic natural cubic spline for sampled-data deprojection.
utils/Test/CubicSpline.H	Declares the cubic spline used by `Deprojector` sampled-data ctor.
utils/Test/CMakeLists.txt	Registers the new `utils/Test` executables in the build.
include/EmpCylSL.H	Adds AbelType state to EmpCylSL for selecting deprojection variant.
include/DiskModels.H	Adds a new `Toomre` disk density model (used for deprojection conditioning).
exputil/EmpCylSL.cc	Updates EmpCylSL’s Abel inversion integral to support multiple variants.
expui/BiorthBasis.cc	Adds `dmodel` string parsing + lookup for selecting deprojection model (mn/exponential/toomre/python).
expui/BiorthBasis.H	Adds `DeprojType` and makes `DiskType` an `enum class` for stronger typing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

include/DiskModels.H

expui/BiorthBasis.cc

utils/Test/Deprojector.cc

utils/Test/testDeproject.cc

utils/Test/testEmp.cc

utils/Test/EmpDeproj.cc

utils/Test/testDeprojPlummer.cc

expui/BiorthBasis.cc

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…n functor for deprojection in EmpCylSL

…on the standalone tests

Copilot

Pull request overview

This PR enhances cylindrical-basis deprojection support by adding a Toomre disk option and improving the numerical Abel inversion implementation, along with new test/verification utilities under utils/Test.

Changes:

Add a Toomre disk model for cylindrical deprojection to provide a less-cuspy alternative to exponential.
Introduce selectable Abel inversion variants (derivative/subtraction/IBP) and change the default to the internal-derivative form.
Add standalone test programs and supporting spline/deprojection utilities for verification.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`expui/BiorthBasis.cc`	Adds `dmodel` string handling and deprojection-model selection (mn/exponential/toomre/python) + `pyproj` config.
`expui/BiorthBasis.H`	Extends Cylindrical config/state with `pyproj`, adds `DeprojType`, and converts `DiskType` to `enum class`.
`exputil/EmpCylSL.cc`	Updates Abel inversion implementation and routes through a new `abel_type` selection.
`include/EmpCylSL.H`	Adds `AbelType` enum + `abel_type` member to control deprojection inversion method.
`include/DiskModels.H`	Adds a `Toomre` disk model usable for deprojection conditioning.
`utils/Test/CMakeLists.txt`	Builds new test executables (`testDeproj`, `testEmpDeproj`, `testEmp`) and links Python embed for `testEmp`.
`utils/Test/testDeproject.cc`	New CLI tool to validate `Deprojector` against analytic profiles.
`utils/Test/testEmpDeproj.cc`	New CLI tool comparing `EmpDeproj` vs `Deprojector` across profiles/Abel variants.
`utils/Test/testEmp.cc`	New CLI tool driving `EmpDeproj`, optionally via embedded Python-provided functions.
`utils/Test/EmpDeproj.cc`	Implements a test-local `EmpDeproj` (EmpCylSL-like) deprojection routine with selectable Abel variants.
`utils/Test/EmpDeproj.H`	Declares the test-local `EmpDeproj` class API.
`utils/Test/Deprojector.cc`	Implements an independent Abel deprojector with improved inner-radius systematics.
`utils/Test/Deprojector.H`	Declares the `Deprojector` API and data-driven/analytic constructors.
`utils/Test/CubicSpline.cc`	Adds a minimal natural cubic spline implementation used by `Deprojector`.
`utils/Test/CubicSpline.H`	Declares the minimal spline interface used by `Deprojector`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

expui/BiorthBasis.cc

    "coefCompute",
    "coefMaster",
    "pyname",
+    "pyproj"


utils/Test/testDeproject.cc

+    RhoFunc = [](double r)->double
+    { return 2.0 / std::pow(1.0 + r*r, 2.0) / M_PI; };
+    break;
+  case Type::Gaussian:
+    // Test function
+    SigmaFunc = [](double R)->double
+    { return exp(-0.5*R*R); };
+    // Analytic derivative
+    dSigmaFunc = [](double R)->double
+    { return -R*exp(-0.5*R*R); };
+    // Expected result
+    RhoFunc = [](double r)->double
+    { return exp(-0.5*r*r)/sqrt(2.0*M_PI); };
+    break;


utils/Test/testEmpDeproj.cc

+  // Parse command-line options
+  //
+  cxxopts::Options options("testEmpDeproj",
+			   "Test the EmpDeproj class against Deproject"


utils/Test/testEmp.cc

+			   "Test the EmpDeproj class for any Python-supplied density function.\n"
+			   "If the Python module is not supplied,  one of the hard-coded\n"
+			   "functions: Plummer, Gaussian, Toomre may be selected.  The internal\n"
+			   "Abel method: Derivative, Substraction, or IBP may be chosen as well.\n");


utils/Test/CubicSpline.H

+    // set data (x must be strictly increasing)
+    void set_data(const std::vector<double>& x_in, const std::vector<double>& y_in);
+
+    // evaluate spline and its derivative (xx should lie within [xmin(), xmax()], but endpoints are clamped)


utils/Test/testEmpDeproj.cc

+  }
+
+  Deprojector D(SigmaFunc, dSigmaFunc, /*R_data_min=*/0.01, /*R_data_max=*/10.0,
+		/*R_max_extend=*/50.0, /*tail_power=*/-4.0, /*Ngrid=*/6000);


utils/Test/testEmpDeproj.cc

+    double t = (double)i / (Nr - 1);
+    r_eval.push_back(0.01 + t * 8.0);


utils/Test/testEmpDeproj.cc

+    // Expected result
+    RhoFunc = [](double r)->double
+    { return 2.0 / std::pow(1.0 + r*r, 2.0) / M_PI; };
+    break;
+  case Type::Gaussian:
+    // Test function
+    SigmaFunc = [](double R)->double
+    { return exp(-0.5*R*R); };
+    // Analytic derivative
+    dSigmaFunc = [](double R)->double
+    { return -R*exp(-0.5*R*R); };
+    // Expected result
+    RhoFunc = [](double r)->double
+    { return exp(-0.5*r*r)/sqrt(2.0*M_PI); };
+    break;


utils/Test/EmpDeproj.cc

+  Linear1d surf(rl, sigI);
+  surfRg = surf;
+
+  // Now, compute Abel inverion integral


utils/Test/EmpDeproj.cc

+  std::vector<double> rho(NUMR), mass(NUMR);
+  for (int i=0; i<NUMR; i++) {
+    if (type == AbelType::IBP)
+      rho[i] = -intgr.deriv(rl[i])/rr[i]/M_PI;
+    else
+      rho[i] = -rhoI[i]/M_PI;
+  }


The9Cat and others added 5 commits March 12, 2026 10:00

Added a Toomre-type disk for deprojection primarily

494c619

Added an enum and reflector for deprojection disk types and included …

c7bf5ee

…the Toomre model in the updated list of valid types

Addded a test routine for Abel deprojection; updated EmpCylSL for der…

6efc024

…ivative form, which is more accurate near the center

Missing source driver file

22adb60

Missing test class

b12a1f8

The9Cat requested a review from Copilot March 13, 2026 16:26

Copilot started reviewing on behalf of The9Cat March 13, 2026 16:27 View session

The9Cat changed the title ~~Toomre~~ BiorthBasis::Cylinder deprojection improvements Mar 13, 2026

Copilot AI reviewed Mar 13, 2026

View reviewed changes

The9Cat and others added 8 commits March 13, 2026 12:42

Potential fix for pull request finding

3cf4d6c

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

a03bbe6

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

4042544

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

db0e02d

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

dfc99f9

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

2e6fee4

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Add a test routine from EmpDeproj alone; fix parsing error in testEmp…

204d43d

…Deproj for method variables

Updated parsing in BiorthBasis::Cylindrical for deprojection

0e82cb6

The9Cat requested a review from Copilot March 13, 2026 17:06

Copilot started reviewing on behalf of The9Cat March 13, 2026 17:10 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

The9Cat and others added 10 commits March 13, 2026 14:32

Potential fix for pull request finding

cb5e150

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

5f151bc

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

b761050

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

25f893d

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

f9a6ae2

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

5a66623

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

4175b1e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

31dad72

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

9c2f36f

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Use new sech^2 scaling

22b6493

Merge branch 'Toomre' of github.com:EXP-code/EXP into Toomre

84bef8f

The9Cat marked this pull request as draft March 13, 2026 22:01

Martin D. Weinberg and others added 5 commits March 13, 2026 19:39

Python deprojection test routine; add possibility of a separate Pytho…

9439daa

…n functor for deprojection in EmpCylSL

Fix typo in parameter assignment

1d50ae5

Remove the initial test function; add doc strings to the help stanza …

eb1f9eb

…on the standalone tests

Only print deprojection info from one node

6402d49

Merge branch 'devel' into Toomre

5b0aa7d

The9Cat requested a review from Copilot March 14, 2026 17:27

Copilot started reviewing on behalf of The9Cat March 14, 2026 17:27 View session

Copilot AI reviewed Mar 14, 2026

View reviewed changes

		double t = (double)i / (Nr - 1);
		r_eval.push_back(0.01 + t * 8.0);

Conversation

The9Cat commented Mar 13, 2026

Movitation

Improvements

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants