Coordinate system issues: cylindricalCS / fixedProfileFvPatchField

• cyclindricalCS cannot be run-time selected: missing TypeName and addToRunTimeSelectionTable.
• hard to specify

        coordinateSystem
        {
            coordinateRotation
            {
                type            axesRotation;
                e3              (1 0 0);
                e1              (0 1 0);
            }
            type            cylindricalCS;
            origin          (0 0 0.01);
        }

With this in place we could re-write various boundary conditions to use cylindrical coordinates. E.g. fixedProfileFvPatchField.

@matej

Remark: cylindrical coordinateRotation currently stores transformation tensors. This should be stored outside.

Or have Function2, Function3 and directly obtain a value from a (set of) 2D/3D coordinate(s).

This could ultimately replace e.g. timeVaryingMappedFixedValue.

changed the description

changed title from fixedProfileFvPatchField is 1D in cartesion coords only. Why not e.g. axisymmetric to Coordinate system issues: cylindricalCS / fixedProfileFvPatchField

changed the description

added ~15 label

coordinate system cannot be re-read. It requires keyword coordinateRotation which does not get written. Writing gets done with:

    os.writeKeyword("coordinateSystem");
    coordSys_->writeDict(os, true);

• type gets written as coordinateSystem instead of cylindrical
• there is no coordinateRotation section

coordinateSystem::New(const dictionary&) doesn't use the run-time selection system so is only good for constructing a coordinateRotation.

While fixing #884 (closed), I started taking a more general re-visit of coordinate systems. I have a local branch with various bits of refactoring that will cleanup a fair amount of other junk.

I also noticed the output you mention (which really does look quite dubious), but held off making any radical changes at all for 1806.

I believe that the coordinateSystem::New(const dictionary&) is a holdover from when the globally referenced coordinate systems got put in and we changed to the two-parameter form. At some point, these were also supposed to work with having type being optional and defaulting to the equivalent of cartesian.

More: coordinateSystem has non-virtual clone(), writeDict(). clone() uses default copy constructor so does not copy coordinateRotation (since autoPtr steals it). Hence cannot be used in e.g. bc in decomposePar.

This is one of the issues that I detected with autoPtr. This is the main reason std::auto_ptr was deprecated and replaced with std::unique_ptr that only allows a move construct and move assignment.

If we undefine these

// Transitional features/misfeatures:
#define Foam_autoPtr_copyConstruct
#define Foam_autoPtr_copyAssign
#define Foam_autoPtr_castOperator

we'd see that things like coordinateSystem etc have default copy constructors, which implicitly call the autoPtr copy (and steal) constructor.

IMO stealing should be disallowed, but this means that all classes that currently have an autoPtr as a member will to explicitly define a copy constructor.

mentioned in commit 8a9db156924ccfcb7170f61a9b4ceddcf37a5cfa

mentioned in commit f34edc6280631bb7519a8ac19f1e2264f2904377

mentioned in commit 1a7cc7144665322f64bf193dba13f1a4c2665931

mentioned in commit 3c3cf51f6977dd110d9932e10e5a0d6b45190342

Conclusions from discussion yesterday with @Mattijs about !211 (merged)

1. The split of coordinate system into the origin (coordinateSystem) and orientation (coordinateRotation) isn't really need. We only need a runtime means of specifying the orientation. So move the rotation matrix to be a direct part of coordinateSystem and its associated transformations.
2. Position-dependent transformations (eg, a vector at some location in a cylindrical coordinate system) belong in the corresponding coordinate system (eg, cylindricalCS).

Discussion today with @andy.

• OK with the above points, but for things like rotorDiskSource that are called often, should provide a caching mechanism for the position-dependent rotation tensors. The caching may need to be realized via some fancy sub-class instance or equivalent with a capture of a given pointField reference. The current coordinateRotation has both R() and Rtr(), but we can eliminate one of these by simply using the opposite tensor-vector, vector-tensor inner product instead.

caching mechanism: can we see if it goes without. For most cylindrical transformations we don't need a tensor but can just use a crossproduct instead.

mentioned in commit 301ffe43823c2142331e7250655a312188c3a60f

mentioned in commit 0d3bcd1bcb05366ba414108192916c54ca73a487

mentioned in commit e338c11a46a223bbbdd818d76d20cef07adc89ba

mentioned in commit 5363766926f829f306f7f0940b23de6876b78709

mentioned in commit 331e53b57dc90e065467675c784bffe08febaa1a

mentioned in commit a43626054bd72b68e285e6ff6d22ea8555adfb03

mentioned in commit 00a509ea0e5b057464bc3f7c0b4fba1d7fce147f

mentioned in commit a947ee23a00ee0cce601983e8c6da5e9ecfd24b1

mentioned in commit fb13c6afcd570036be510b0ac846f6e3019fc60f

mentioned in commit 065a20897073293665b44f5d9d907dc3e17e5305

mentioned in commit 0d792cd516e39f2c724764663ee3c12ee76a24d4

mentioned in commit c75d511858946aa9874dcfe228efc55c2707da74

mentioned in commit e2182625baf54d6a9c5994743aac12e3f7aae48a

mentioned in commit 07e29aef2281b39661a5bad6f4f580e6a84abfd3

mentioned in commit 4b338a33249806861426f8fd7584822689a9206f

mentioned in commit b608f90e64fb55825917eb06bc71f6a8c2c2f09c

mentioned in commit cbf9417542c967e2cbaaffbd116e9b852226baa1

mentioned in commit e9acb89a8c8b9ed1a4c0baaf297a20596786f60e

This should be solved with all of the coordinate-system updates from last year.

closed