Skip to content

Add OneD namespace - #2143

Open
ischoegl wants to merge 7 commits into
Cantera:mainfrom
ischoegl:OneD-namespace
Open

Add OneD namespace#2143
ischoegl wants to merge 7 commits into
Cantera:mainfrom
ischoegl:OneD-namespace

Conversation

@ischoegl

@ischoegl ischoegl commented Jun 21, 2026

Copy link
Copy Markdown
Member

Changes proposed in this pull request

This pull request introduces the nested Cantera::OneD C++ namespace for the 1D solver suite and updates Python (Cython), the sourcegen code generator (CLib wrappers), and the C++ samples and tests accordingly. The change mimics the recently introduced oneD namespace of the MATLAB API. If adopted, a Cantera::ZeroD namespace will be introduced in a separate PR.

  • C++ Namespace Refactoring: Moved all 1D core headers and implementations (under include/cantera/oneD/ and src/oneD/) into the nested Cantera::OneD namespace. Affected classes/functions include Domain1D, Empty1D, Sim1D, Flow1D, IonFlow, OneDim, and refine.
  • Cython Python Bindings Update: Updated Cython definitions in interfaces/cython/cantera/_onedim.pxd to reference the new C++ namespace Cantera::OneD:: for underlying 1D C++ type mappings.
  • Sourcegen Doxygen Parser Update: Modified the TagInfo parsing logic in interfaces/sourcegen/src/sourcegen/headers/tagfiles.py to strip namespace and class prefixes from XML member names.
  • Sourcegen Class Method Detection: Updated the method wrapper generator in interfaces/sourcegen/src/sourcegen/headers/generator.py to keep track of all wrapped classes (both base classes and derived classes like Boundary1D and Flow1D). Distinguishes nested namespace-level constructors (e.g. OneD::newFlow1D) from class methods, ensuring correct parameter signature scaffolding (avoiding incorrect handle argument generation).
  • C++ Template Scope Forwarding: Forward-declared the Cantera::OneD namespace inside interfaces/sourcegen/src/sourcegen/clib/template_source.cpp.j2 before declaring using namespace Cantera::OneD;. This prevents compiler errors in generated translation units that do not include 1D headers.
  • Unit Test and Sample Updates: Updated test/oneD/test_oneD.cpp, samples/cxx/bvp/BoundaryValueProblem.h, and samples/cxx/flamespeed/flamespeed.cpp to explicitly qualify references with OneD:: (e.g. OneD::newFlow1D, OneD::Sim1D, OneD::Domain1D).

Note: I did not issue deprecation warnings yet as this is a major version jump. Further, it is evident that the 1D suffix can be removed from various class names after the change (e.g. Domain1D becomes OneD::Domain). As this would increase the diff substantially, it would be added as a follow-up PR.

If applicable, provide an example illustrating new features this pull request is introducing

A minimal example of a C++ application using the new Cantera::OneD namespace to resolve and assemble 1D flow domains:

#include "cantera/onedim.h"
#include "cantera/oneD/DomainFactory.h"

using namespace Cantera;

int main() {
    auto sol = newSolution("gri30.yaml", "gri30", "mixture-averaged");

    // Instantiating 1D domains using the factory function in OneD namespace explicitly
    auto flow = OneD::newFlow1D("gas-flow", sol, "flow");
    flow->setFreeFlow();

    auto inlet = OneD::newDomain<OneD::Inlet1D>("inlet", sol);
    auto outlet = OneD::newDomain<OneD::Outlet1D>("outlet", sol);

    // Assembly using OneD namespace Domain1D and Sim1D container
    std::vector<std::shared_ptr<OneD::Domain1D>> domains { inlet, flow, outlet };
    OneD::Sim1D flame(domains);

    return 0;
}

AI Statement (required)

  • Extensive use of generative AI. Significant portions of code or documentation were generated with AI, including logic and implementation decisions. All generated code and documentation were reviewed and understood by the contributor. The initial Cantera::OneD namespace was drafted without AI, while sourcegen updates were outsourced (implementation by Antigravity / Gemini 3.5 Flash with feedback from Codex / gpt-5.5).

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • AI Statement is included
  • The pull request is ready for review

@codecov

codecov Bot commented Jun 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.15%. Comparing base (39cfc14) to head (2af552d).
⚠️ Report is 49 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2143   +/-   ##
=======================================
  Coverage   78.15%   78.15%           
=======================================
  Files         453      453           
  Lines       55433    55433           
  Branches     9117     9117           
=======================================
  Hits        43324    43324           
  Misses       9060     9060           
  Partials     3049     3049           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ischoegl
ischoegl marked this pull request as ready for review June 21, 2026 19:14
@ischoegl
ischoegl requested a review from a team June 21, 2026 19:14
@ischoegl

ischoegl commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

Rebased after #2139.

As mentioned on top, I am ready to add a namespace to ZeroD as well. I did not issue deprecation warnings yet as this is a major version jump, but it's easy to add. Further, it is evident that the 1D suffix can be removed from various class names after the change (e.g. Domain1D becomes OneD::Domain). As this would increase the diff substantially, it would be added as a follow-up PR or after an initial 👍 here.

@speth

speth commented Jul 8, 2026

Copy link
Copy Markdown
Member

For context, how do you see this with respect to Cantera/enhancements153 -- as a step along the way to that split, or as an alternative, or something else?

@ischoegl

ischoegl commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

For context, how do you see this with respect to Cantera/enhancements153 -- as a step along the way to that split, or as an alternative, or something else?

@speth ... this is aligned with Cantera/enhancements#153, and stopping along the way; I don't think that this has to be split it into a separate library. My main concern is to establish a clear separation of core and applications.

@ischoegl

Copy link
Copy Markdown
Member Author

@speth - I rebased after the merge of #2135 (no changes were necessary). What's your take on the nested namespace? As noted above, my suggestion is to establish a clearer separation between core and applications.

@speth speth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The source code reorganization to move MultiJac and MultiNewton out of oneD and into numerics fixes a clear wart that was created when these were generalized to be usable by both the 1D and 0D solvers.

Rather than moving the existing classes/methods into a namespace and requiring changes to all existing uses of these names, I'd like to suggest a lighter alternative that still provides an opportunity to expose an organized view of this module, which would be to create aliases of these types in a Cantera::OneD namespace while leaving the actual classes where they are now. For example, the following could be added to onedim.h:

namespace Cantera::OneD
{
using Domain = ::Cantera::Domain1D;
using Flow = ::Cantera::Flow1D;
using ::Cantera::IonFlow;
...
}

This would eliminate the need for the extra complexity that nested namespaces seems to be adding to sourcegen (clib/generator.py now knows about OneD, even though it otherwise knows almost nothing specifically about Cantera).

I'm still struggling a bit with the rationale for this reorganization, and even moreso with the expected benefit of what's proposed in Cantera/enhancements#153. I think we need to discuss that in detail before pursuing anything more than adding some optional namespaced aliases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants