Add postprocessing of lumped port boundaries to eigenmode simulations #129

sebastiangrimberg · 2023-11-09T23:38:21Z

These quantities can be useful in addition to the EPR output for characterizing multi-port systems along with the computed pole locations.

This PR also updates the output of various computed quantities like energies, voltages, currents, and field values to be dimensional based on a choice for H0, the characteristic magnetic field strength used for nondimensionalization (see https://awslabs.github.io/palace/stable/reference/). In particular, we choose H0 so that the unit power incident on ports for driven simulations corresponds to 1 W. The fields written for ParaView visualization are NOT redimensionalized, however. They can be if this would be useful, but for now we leave them nondimensional since they are only used for visualization.

Resolves #123.

hughcars · 2023-11-14T15:28:38Z

palace/drivers/basesolver.cpp

@@ -428,6 +432,8 @@ void BaseSolver::PostprocessProbes(const PostOperator &postop, const std::string
      i++;
    }
    const std::string F = (f == 0) ? "E" : "B";
+    const std::string unit = (f == 0) ? "(C/m²)" : "(Wb/m²)";
+    const auto type = (f == 0) ? IoData::ValueType::FIELD_E : IoData::ValueType::FIELD_B;


Minor: Given f is only being used for f==0 inside the loop, could use something like for (auto type : {IoData::ValueType::FIELD_E, IoData::ValueType::FIELD_B}) as the loop variable, and check off that directly?

Sure, could do that too. Doesn't matter either way and will leave as is.

hughcars · 2023-11-14T15:42:29Z

docs/src/guide/postprocessing.md

@@ -14,6 +14,10 @@ element parameters. In addition, each simulation type will write a file called
 as well as lumped element energies, for each step of the simulation (eigenmode, frequency,
 or time step, for examples).

+Models containing ports lumped or wave port boundaries or surface current excitations will


lumped ports rather than ports lumped?

hughcars · 2023-11-14T15:44:41Z

docs/src/guide/postprocessing.md

+each step of the simulation (eigenmode, frequency, or time step) are written to ASCII files
+named `port-V.csv` and `port-I.csv`, respectively. These files also include the excitation
+voltage and current corresponding to the incident wave on excited port boundaries.
+


Given the "Additionally" this seems like it should be in the previous paragraph?

I think it's fine as is, separated to emphasize ports vs. surface currents.

hughcars · 2023-11-14T15:49:17Z

docs/src/guide/problem.md

-the directory specified by [`config["Problem"]["Output"]`]
-(../config/problem.md#config["Problem"]). Additionally, surface current excitation time
-histories are written to `surface-I.csv`.
+As for the frequency domain driven case, postprocessing of quantities related to ports is


Suggestion: Postprocessing of quantities related to ports for the frequency domain driven case is described in the sections...

This change would not be correct and would change the meaning of the sentence. This is referring to time domain port processing.

Then it would be "As with"? As for is describing the frequency domain, as with is drawing similarity to the frequency domain case.

Alright, I'll change to "As with" and you can count it as a win for the grammar police.

hughcars · 2023-11-14T15:53:27Z

palace/drivers/drivensolver.cpp

-      Mpi::Print(" Field energy E ({:.3e}) + H ({:.3e}) = {:.3e}\n", E_elec, E_mag,
-                 E_elec + E_mag);
+      Mpi::Print(" Field energy E ({:.3e} J) + H ({:.3e} J) = {:.3e} J\n", E_elec * J,
+                 E_mag * J, (E_elec + E_mag) * J);


In other places (basesolver.cpp) there's a call to iodata.DimensionalizeValue, vs constructing the conversion constant like this. I guess this was done elsewhere because of the clang-format off?

Doesn't really matter, usually use the constant to avoid many repeated calls to DimensionalizeValue but it's a choice without much difference either way.

hughcars · 2023-11-14T16:05:47Z

palace/drivers/basesolver.cpp

@@ -428,6 +432,8 @@ void BaseSolver::PostprocessProbes(const PostOperator &postop, const std::string
      i++;
    }
    const std::string F = (f == 0) ? "E" : "B";
+    const std::string unit = (f == 0) ? "(C/m²)" : "(Wb/m²)";


These units don't match those from IoData::DimensionalizeValue, where ValueType::FIELD_E is [V/m] and ValueType::FIELD_B is [Wb/m²]. Which is correct or am I missing a unit conversion here?

Yep, should be V/m for E. Was distracted doing units of B instead of H and so wrote units for D instead of E. Good catch.

hughcars · 2023-11-14T16:06:59Z

palace/utils/iodata.cpp

  }
  return v * sf;
 }

+template double IoData::DimensionalizeValue(IoData::ValueType, double) const;


Given the explicit instantiation of only the double case and this being in a cpp, why the template? I'm guessing that complex was going to be an option originally?

hughcars · 2023-11-14T16:08:36Z

palace/utils/iodata.cpp

@@ -556,10 +556,13 @@ void IoData::NondimensionalizeInputs(mfem::ParMesh &mesh)
             Lc, tc);
 }

-double IoData::DimensionalizeValue(IoData::ValueType type, double v) const
+template <typename T>


Given this is a free function, could probably be marked constexpr if std::sqrt was. I'm assuming that there's some compilers we use that don't support that though, which is preventing that change? Would make the cases with the fixed conversion constants possible to be constexpr too.

std::sqrt is not always constexpr and you don't gain much (anything) here from this being constexpr. Fine as is.

hughcars · 2023-11-14T16:11:11Z

palace/utils/iodata.hpp

+  template <typename T>
+  T DimensionalizeValue(ValueType type, T v) const;
+  template <typename T>
+  std::complex<T> DimensionalizeValue(ValueType type, std::complex<T> v) const
+  {
+    return {DimensionalizeValue(type, v.real()), DimensionalizeValue(type, v.imag())};
+  }


I think the templates here can all be removed for double given the base case is only instantiated for double in the cpp. I believe if you passed DimensionalizeValue(type, 1); with this, it would compile and then linker error, whereas with explicit double it would perform the implicit conversion which is what would be wanted. If the goal is to support variable precision, I think that should be done in another pr upgrading all the usages of double.

Commenting here along with #129 (comment), but yes originally I had templated for complex but then decided on this design being nicer. I don't see any penalty for it being templated, so will keep it. If someone is converting an integral type they can add an explicit instantiation when they come upon the linker error.

The function doesn't however work for other types because all the constants in electromagnetics are double. Adding an explicit instantiation for int for instance gives

const auto f1 = iodata.DimensionalizeValue(IoData::ValueType::FREQUENCY, 1.0); const auto f2 = iodata.DimensionalizeValue(IoData::ValueType::FREQUENCY, 1); std::cout << "f1 " << f1 << " f2 " << f2 << std::endl;

prints f1 0.870683 f2 0 due to the rules on integer multiplication/division and casting. Dropping the template for double performs the conversions and gives f1 0.870683 f2 0.870683 as expected. The extra template syntax buys unexpected behaviour, or requires a user to exercise extra care on numeric types, neither seems ideal.

OK maybe it was a bad quick example, you'd never want to instantiate this function for an int and if you did then the output you are getting is actually exactly what you'd expect and I argue is actually correct behavior (the returned type should be integral, not floating point in this case). I have a hard time seeing where developers will not be able to take the care to work with the function as it currently exists.

hughcars · 2023-11-14T16:30:21Z

palace/drivers/eigensolver.cpp

+        output.print("{:+{}.{}e},{:+{}.{}e}{}",
+                     data.Vi.real(), table.w, table.p,
+                     data.Vi.imag(), table.w, table.p,
+                     (data.idx == port_data.back().idx) ? "" : ",");


There are no repeated port indices correct?

That's right, but this is a check to skip the comma at the last iteration of the loop.

… and correct redimensionalization of voltages (V), currents (I) for output

…ning

… output

sebastiangrimberg · 2023-11-16T19:04:27Z

@hughcars, any final comments here that need to be addressed before approval?

sebastiangrimberg added enhancement New feature or request eigenmode Related to eigenmode simulations labels Nov 9, 2023

sebastiangrimberg requested a review from hughcars November 13, 2023 20:22

sebastiangrimberg force-pushed the sjg/eigenmode-port-postpro branch from b55057f to 3d2c39f Compare November 13, 2023 21:28

hughcars reviewed Nov 14, 2023

View reviewed changes

sebastiangrimberg added 6 commits November 14, 2023 08:41

Lumped port voltage/current postprocessing for eigenmode simulations,…

fc5241a

… and correct redimensionalization of voltages (V), currents (I) for output

Dimensionalize output of energy integrals and probes

839d736

Update docs and changelog

fe6622f

Update regression testing framework to add simple helper for rebaseli…

8610bbd

…ning

Rebaseline regression examples for new formatting and dimensionalized…

88ea1de

… output

Address PR comments

5198e05

sebastiangrimberg force-pushed the sjg/eigenmode-port-postpro branch from 3d2c39f to 5198e05 Compare November 14, 2023 17:05

Grammatical fix for docs

92db4d0

sebastiangrimberg force-pushed the sjg/eigenmode-port-postpro branch 3 times, most recently from 77d0715 to e313a33 Compare November 14, 2023 21:32

Attempt to fix Intel CI issue and PETSc build

59bd14f

sebastiangrimberg force-pushed the sjg/eigenmode-port-postpro branch from e313a33 to 59bd14f Compare November 14, 2023 22:55

sebastiangrimberg mentioned this pull request Nov 16, 2023

Add bulk magnetic field energy postprocessing #131

Merged

hughcars approved these changes Nov 16, 2023

View reviewed changes

sebastiangrimberg merged commit 4156d13 into main Nov 16, 2023
17 checks passed

sebastiangrimberg deleted the sjg/eigenmode-port-postpro branch November 16, 2023 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add postprocessing of lumped port boundaries to eigenmode simulations #129

Add postprocessing of lumped port boundaries to eigenmode simulations #129

sebastiangrimberg commented Nov 9, 2023 •

edited

Loading

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023

hughcars Nov 14, 2023

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023 •

edited

Loading

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023

hughcars Nov 14, 2023

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023

hughcars Nov 14, 2023

sebastiangrimberg Nov 14, 2023 •

edited

Loading

sebastiangrimberg commented Nov 16, 2023

Add postprocessing of lumped port boundaries to eigenmode simulations #129

Add postprocessing of lumped port boundaries to eigenmode simulations #129

Conversation

sebastiangrimberg commented Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastiangrimberg Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastiangrimberg Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

sebastiangrimberg commented Nov 16, 2023

sebastiangrimberg commented Nov 9, 2023 •

edited

Loading

sebastiangrimberg Nov 14, 2023 •

edited

Loading

sebastiangrimberg Nov 14, 2023 •

edited

Loading