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

Draft: Print geometry path in case of e.g. GSL error during calculation #117

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

2xB
Copy link
Member

@2xB 2xB commented Jul 8, 2024

This introduces new path information in case of an error during KEMField calculations. Example (the second-to-last line is added by this PR):

[ERROR] l/Utility/KGslErrorHandler.cxx:84    GSL error: ellint.c:542: domain error (1)
  1# gsl_sf_ellint_Ecomp_e
  2# gsl_sf_ellint_Ecomp
  3# KEMField::KElectrostaticAnalyticRingIntegrator::EFieldRFromChargedRing(double const*, double const*)
  4# KEMField::KGaussianQuadrature::operator()(double (**)(double const*, double const*), int, double, double, double*, int, double*) const
  5# KEMField::KElectrostaticAnalyticConicSectionIntegrator::ElectricField(KEMField::KConicSection const*, KEMField::KThreeVector_<true> const&) const
[KEMFIELD WARNING MESSAGE] Error in <ConicSection<world/axial_main_spec/upstream_beam_tube>>
****************[KSSTEP WARNING MESSAGE] Failed to execute step <11785> (GSL error: ellint.c:542: domain error (1))

However, support with testing and potentially extending this PR would be great, since while this is very useful for debugging e.g. bugs like #69 , in its current implementation it may have a few potential downsides:

  1. This does only work when not using KEMField caches or when the cache is empty.
  2. (untested) Every geometry element now has a long string - the path - associated. This may potentially lead to significantly more memory usage for very large meshes (when no cache is used or the cache is empty).

Regarding 1., there are two potential solutions to keeping this feature when loading geometry from a cache: Loading the geometry from the cache for coefficients but keeping path information from a freshly built version of the geometry or storing and loading the path information as well - probably the former is more reasonable.

Regarding 2., if this actually turns out to be a problem, the solution could be to link the original KGPathAware fCurrentElement to each KShape as a pointer to dynamically get the path instead of having it saved as a long string, which however introduces a dependency between KEMField and KGeoBag also outside the KGeoBag interface. It has to be noted that geometry elements already contain strings in the location that is now used for storing the path information, which may or may not already be optimizable. The corresponding name (passed to KShapes using KNamed::SetName in KGBEMConverter) however is probably never used (KShapes usually overwrite Name(); KSurface, which is used to wrap the KShapes in KGBEMConverter, overwrites GetName() to call Name(), so the name set with SetName is probably inaccessible for objects created by KGBEMConverter).

2xB added 2 commits July 9, 2024 00:14
`fCurrentElement` is e.g. used to extract path information
from the currently active space/surface. Before this commit,
it was oftentimes not set consistently and was not always
unset after it was processed, potentially causing unintended
side effects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant