Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce arbitrary orientation for lumped ports #75

Merged
merged 21 commits into from
Aug 22, 2023

Conversation

hughcars
Copy link
Collaborator

@hughcars hughcars commented Jul 20, 2023

Description of changes:

  • Introduces the ability to specify the orientation of a lumped port using a 3 vector [x,y,z], in addition to the existing key word "X", "-Y", "+R" style interface.
  • Introduces algorithms for computing BoundingBox and BoundingBall. These work by reducing onto the dominant rank all data associated with a marked portion of the mesh, then applying sorting/separation approaches based on a a highly restricted subset of the QuickHull algorithm.
  • The rings example has been changed to apply a π/6 around (0,0,1). This requires regeneration of the baseline data, and changing of the probe example location and data.

Nuances:

  • The choice to avoid a full QuickHull implementation or Chan's algorithm (generalizes across processors) was based on pragmatism. The upside is relative simplicity (each algorithm requires at most two projections and distance measurements, and some lexicographic sorting) but at the cost of applying a serial bottle neck. This was viewed as not a practical issue however, as generally a) the number of vertices in a port is small and b) most of these vertices are on one rank. If however there was a port that had many vertices, and was uniformly split amongst processors, this would be problematic for these algorithms.
  • Though these algorithms will work for 3D objects, they should not in general be used for large (made up of many vertices) objects, due to the reduction potentially placing a large quantity of data on the dominant rank (the rank with the largest number of initial vertices).
  • Usage of the bounding box algorithms in lumpedelement.hpp is to decide upon a length/width of a rectangular port, and outer radius of a coaxial port.
    • The appropriate specification of the "+R" direction in a coaxial port is non ambiguous, however the specification of circumferential or azimuthal forcing would be ambiguous. This ambiguity was not resolved, as at present neither of these forcing types have been needed.

Issue #, if available:
Closes #71

@hughcars hughcars marked this pull request as draft July 20, 2023 16:23
@hughcars hughcars force-pushed the hughcars/lumped-port-orientation-dev branch 3 times, most recently from 4ef850e to 5c195ec Compare July 20, 2023 17:42
@sebastiangrimberg sebastiangrimberg added the enhancement New feature or request label Jul 20, 2023
@hughcars hughcars marked this pull request as ready for review July 20, 2023 19:11
@hughcars hughcars marked this pull request as draft July 20, 2023 20:35
@hughcars
Copy link
Collaborator Author

Reverting to draft after discussion about bug in the bounding box length calculation.

@hughcars hughcars force-pushed the hughcars/lumped-port-orientation-dev branch from 5c195ec to 0016dfb Compare August 3, 2023 18:25
@hughcars hughcars force-pushed the hughcars/lumped-port-orientation-dev branch 2 times, most recently from ae7b253 to 6e8c8fc Compare August 8, 2023 20:05
@hughcars hughcars changed the base branch from main to sjg/driven-port-debug August 8, 2023 20:05
@hughcars hughcars force-pushed the hughcars/lumped-port-orientation-dev branch from 6e8c8fc to e9ea850 Compare August 8, 2023 21:27
Copy link
Contributor

@sebastiangrimberg sebastiangrimberg left a comment

Choose a reason for hiding this comment

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

Cool stuff! Thanks @hughcars.

docs/src/config/boundaries.md Outdated Show resolved Hide resolved
docs/src/config/boundaries.md Outdated Show resolved Hide resolved
docs/src/config/boundaries.md Outdated Show resolved Hide resolved
examples/coaxial/coaxial_matched.json Outdated Show resolved Hide resolved
examples/cpw/cpw_lumped_uniform.json Outdated Show resolved Hide resolved
palace/utils/communication.hpp Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
palace/utils/geodata.cpp Outdated Show resolved Hide resolved
palace/utils/geodata.cpp Outdated Show resolved Hide resolved
palace/utils/geodata.cpp Outdated Show resolved Hide resolved
@hughcars hughcars force-pushed the hughcars/lumped-port-orientation-dev branch from e9ea850 to 4855f20 Compare August 10, 2023 19:05
examples/rings/rings.json Outdated Show resolved Hide resolved
@hughcars hughcars force-pushed the hughcars/lumped-port-orientation-dev branch 3 times, most recently from e042081 to 1f611a9 Compare August 10, 2023 20:39
@hughcars hughcars force-pushed the hughcars/lumped-port-orientation-dev branch from 2311224 to 51a930d Compare August 10, 2023 23:16
@sebastiangrimberg
Copy link
Contributor

sebastiangrimberg commented Aug 10, 2023

I've pushed a branch sjg/lumped-port-orientation-fixes which contains some final fixes (almost all style/formatting) to this branch. If it looks good to you, feel free to merge it here before this ready for merge (following #75 on which it is based).

@sebastiangrimberg sebastiangrimberg force-pushed the hughcars/lumped-port-orientation-dev branch 2 times, most recently from 452ba2f to 8a1aebf Compare August 11, 2023 17:44
hughcars and others added 19 commits August 22, 2023 10:16
* Both make use of a reduce/gather paradigm to compute bounding boxes/balls
for a collection of points. Given this, they do not scale well with growing ranks.
They are however capable of handling arbitrary orientation of two dimensional ports.
* The algorithms rely on sorting tricks applied to containers of the vertices
* The uniform lumped port was tested by applying a 30 degree rotation about all three axes
to all the vertices of the rings mesh.
* The uniform lumped port also performs an axis snapping for user specified direction vectors,
this means the resulting calculations are forgiving on precision issues.
* Calculations were tested by applying rotations to the global mesh and checking that the resulting areas agree with
the integrated values
…tes from a discovered axis, rather than snap to axis
…0,1).

- Most of the values are perturbed only on the 3rd sf or so.
- For the probe data, it is necessary to apply the axis angle transformation on the original data
  - In julia:

```
julia> AngleAxis(pi/3, 0,0,1) * [-6.975740032e-01,        -3.331547360e+00,        +1.022439079e+02]
3-element StaticArraysCore.SVector{3, Float64} with indices SOneTo(3):
   2.5364176460709804
  -2.269890487790808
 102.2439079

julia> AngleAxis(pi/3, 0,0,1) * [-2.108257694e-03,        -1.094675073e-02,        +1.007018922e+01]
3-element StaticArraysCore.SVector{3, Float64} with indices SOneTo(3):
  0.008426035374075847
 -0.007299180085728002
 10.07018922
```
which agree fairly closely with the newly generated values
```
            i,                  B_x[1],                  B_y[1],                  B_z[1]
 1.000000e+00,        -2.744244415e+00,        +2.362252272e+00,        +1.021127464e+02
 2.000000e+00,        -8.553328102e-03,        +7.180956729e-03,        +1.007140281e+01
```
- All the non-probe data is very close to the original values, modulo a few sf
- The probe data requires a transformation of the rebaselined data:
```
julia> Ref(AngleAxis(pi/6, 0,0,1)) .* [[-1.934600007e+00, +1.332005966e+00, +1.007689041e+02],
				       [-2.056769746e-03, +3.073509047e-03, +1.007227123e+01]]
2-element Vector{StaticArraysCore.SVector{3, Float64}}:
 [-2.3414157352235527, 0.18625100104843154, 100.7689041]
 [-0.0033179693732712672, 0.0016333520404633007, 10.07227123]
```
which compares to
```
            i,                  B_x[1],                  B_y[1],                  B_z[1]
 1.000000e+00,        -2.471968830e+00,        +3.021551217e-01,        +1.012769086e+02
 2.000000e+00,        -7.789873394e-03,        +1.048294735e-03,        +1.006816729e+01
```
the additional discrepancy can be partially attributed to breaking of rotational symmetry, as the axis
does not pass through a vertex of the bounding box of the domain. Better agreement can be found with
pi/3 around (1,1,1), but (0,0,1) is chosen for ease of understanding in the examples.

Note: the data was generated on a mac M1 with a tighter tolerance of 1e-9, as this was required to reduce discrepancy
in the values of the second probe and to agree with CI.
- Update schema to account for CoordinateSystem
- Fix bugs with config parsing and update some more config files
- Refactor point cloud calculations to reduce duplication and hide local functions
…nd comments and allowing GlobalMinLoc/MaxLoc to use arbitrary type for the second argument as supported by the available MPI types
…for maximally separated points to define initial diagonal/diameter
@sebastiangrimberg sebastiangrimberg force-pushed the hughcars/lumped-port-orientation-dev branch from f632ee9 to b9c26d7 Compare August 22, 2023 17:20
@sebastiangrimberg sebastiangrimberg marked this pull request as ready for review August 22, 2023 18:19
@sebastiangrimberg
Copy link
Contributor

@hughcars, I've rebased this on main and it should be ready now. Can you please check out the final commit b9c26d7 which I added to reduce some duplicate code between the BoundingBoxFromPointCloud and BoundingBallFromPointCloud methods? If you're OK with the changes there I will go ahead and mark this for merge.

palace/utils/geodata.cpp Outdated Show resolved Hide resolved
palace/utils/geodata.cpp Outdated Show resolved Hide resolved
palace/utils/geodata.cpp Outdated Show resolved Hide resolved
@hughcars
Copy link
Collaborator Author

@hughcars, I've rebased this on main and it should be ready now. Can you please check out the final commit b9c26d7 which I added to reduce some duplicate code between the BoundingBoxFromPointCloud and BoundingBallFromPointCloud methods? If you're OK with the changes there I will go ahead and mark this for merge.

Looks good, slight change to the comment wording, and two missing const is all.

@sebastiangrimberg sebastiangrimberg merged commit 0cb97ea into main Aug 22, 2023
17 checks passed
@sebastiangrimberg sebastiangrimberg deleted the hughcars/lumped-port-orientation-dev branch August 22, 2023 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow lumped ports to have arbitrary orientation
2 participants