Add OneD namespace - #2143
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
|
Rebased after #2139. As mentioned on top, I am ready to add a namespace to |
|
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. |
Both are now used for oneD and zeroD, so the old oneD location is no longer appropriate.
speth
left a comment
There was a problem hiding this comment.
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.
Changes proposed in this pull request
This pull request introduces the nested
Cantera::OneDC++ 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 introducedoneDnamespace of the MATLAB API. If adopted, aCantera::ZeroDnamespace will be introduced in a separate PR.include/cantera/oneD/andsrc/oneD/) into the nestedCantera::OneDnamespace. Affected classes/functions includeDomain1D,Empty1D,Sim1D,Flow1D,IonFlow,OneDim, andrefine.interfaces/cython/cantera/_onedim.pxdto reference the new C++ namespaceCantera::OneD::for underlying 1D C++ type mappings.TagInfoparsing logic ininterfaces/sourcegen/src/sourcegen/headers/tagfiles.pyto strip namespace and class prefixes from XML member names.interfaces/sourcegen/src/sourcegen/headers/generator.pyto keep track of all wrapped classes (both base classes and derived classes likeBoundary1DandFlow1D). Distinguishes nested namespace-level constructors (e.g.OneD::newFlow1D) from class methods, ensuring correct parameter signature scaffolding (avoiding incorrecthandleargument generation).Cantera::OneDnamespace insideinterfaces/sourcegen/src/sourcegen/clib/template_source.cpp.j2before declaringusing namespace Cantera::OneD;. This prevents compiler errors in generated translation units that do not include 1D headers.test/oneD/test_oneD.cpp,samples/cxx/bvp/BoundaryValueProblem.h, andsamples/cxx/flamespeed/flamespeed.cppto explicitly qualify references withOneD::(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
1Dsuffix can be removed from various class names after the change (e.g.Domain1DbecomesOneD::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::OneDnamespace to resolve and assemble 1D flow domains:AI Statement (required)
Cantera::OneDnamespace was drafted without AI, whilesourcegenupdates were outsourced (implementation by Antigravity / Gemini 3.5 Flash with feedback from Codex / gpt-5.5).Checklist
scons build&scons test) and unit tests address code coverage