SixTrackLib v0.6.0 #132
Merged
SixTrackLib v0.6.0 #132
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Kparasch master
-- Note: changed to use black -l 79 instead of autopep8 to be compatible with the linter script in pysixtrack
This reverts commit e72860a.
python/sixtracklib: fixes wrong calculated offset to TriCubData element
…thon - adds two helper functions to stcommon which encode and decode python (unicode) strings to bytes strings compatible with ctypes.c_char_p - Avoid explicit encoding / decoding in all parts of python/sixtracklib and use the helper functions instead - re-apply the linter after reformating and simplyfing the source code
…klib into kparasch_master_merge
…g handling functions
…lity for common/beam_elements
Kparasch master
…nCL support is disabled
python/sixtracklib: fixes access to OpenCL related C99 symbols if Ope…
…rectly with the ClContext (does not occur in a track-job example)
This attempts to addess comment SixTrack#125 which fails on cc60 There seems to be an issue of typedef conflicts between host code and device code on CUDA (most likely due to the use of uint64_t in common/definitions.h) This is a work-around which should work. The better fix is to transit the SIXTRL_(U)INT(xy) macros to use the proper native types and not to rely on uint64_t etc. being consistent on the device side
…mp (which does not work on OpenCL GPUs)
- makes the path prefix to the kernel/program directory a runtime modifiable attribute of the base controller - this should allow to make the sixtracklib .so / .dll relocatable and address problems with blanks / special chars in the path - adds api to fetch particle addresses on the device
…e address in test
…kJob by changing initialisation order
- The flag CUDA_RESOLVE_DEVICE_SYMBOLS should be enabled by default on shared libraries like SixTrackLib. The property does not work on OBJECT libraries like sixtrack_cuda_device. It seems that starting with cmake 3.15, cmake did not pick up the property for the resulting compound SHARED library just by inferring the dependency on the CUDA device part. This fix attempts to rectify this by adding the property explicitly to the SHARED sixtracklib library if the sixtrack_cuda_device target is present. Tested on CMake 3.14, 3.16 und 3.18 -> seems to work - Note: this would fix issue SixTrack#109 but more investigation is required whether this solves all problems encountered therein
Backported from #PR130 - The flag CUDA_RESOLVE_DEVICE_SYMBOLS should be enabled by default on shared libraries like SixTrackLib. The property does not work on OBJECT libraries like sixtrack_cuda_device. It seems that starting with cmake 3.15, cmake did not pick up the property for the resulting compound SHARED library just by inferring the dependency on the CUDA device part. This fix attempts to rectify this by adding the property explicitly to the SHARED sixtracklib library if the sixtrack_cuda_device target is present. Tested on CMake 3.14, 3.16 und 3.18 -> seems to work - Note: this would fix issue SixTrack#109 but more investigation is required whether this solves all problems encountered therein
By request of users, the map now checks whether the abscissa value for which an interpolation should be calculated is within bounds (as expected) or outside of the range sampled in the abscissa values If the range is exceeded, 0 is returned rather than an assertion or error being propagated. This is a deviation from the previous assumption, that the values *always* have to be within bounds. The performance impact for conforming values should be minimal ( one floating point multiplication, one addition, two comparisons, one bitwise AND) and branch divergence should only occur in very exceptionally few cases. Given the asymptotic behaviour of real-world profiles, this should be a pretty sensible fall-back and avoids hard to debug / trace errors if evaluation very close to the boundaries is required and particles could stray outside of the bounds for example by rounding errors. NOTE: we reserve the right to revert this change in case performance is, unexpectedly, serverly affected. Please report if any unexpected side-effects are associated with this change
Thanks to @kparasch for providing the dataset and the methodology for the testdata!
Thanks to @kparasch for providing the testcase and for the preparation of this test!
- fixes wrong file format qualifier when loading the dumped pickle - casts loaded prng_seed to int as numpy does not accept floating point seeds
- disables the test if the TriCub element is not enabled. - relaxes the error tolerances to rel=1e-12, abs=1e-13 NOTE: Investigate why there is a discrepancy with the dump above the EPS of the machine precision for double?
…into feature/sc_interpol
…ing construction and assignment
NOTE: apparently, m_stored_buffers and m_assign_item_keys are not handled during copy-assignment
operations -> this is not good and should be fixed!
martinschwinzerl
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Main Features:
NS(SpaceChargeQGaussianProfile)andNS(SpaceChargeInterpolatedProfile)beam-elementsNS(LineDensityProfileData)element to store a 1D array of values and derivatives and allows to evaluate these values either using linear or cubic spline interpolated.NS(SpaceChargeInterpolatedProfile)elements can share a singleNS(LineDensityProfileData)instance → cfexamples/python/spacecharge/ line_density_profile.pyfor an example demonstrating the usageNS(SpaceChargeInterpolatedProfile)/NS(LineDensityProfileData)implementation uses the same API as theNS(TriCub)element from Enable/disable features, adds TriCub element (disabled by default), string handling in python #123. Consequently, this PR should be a superset of Enable/disable features, adds TriCub element (disabled by default), string handling in python #123 and will replace itSIXTRL_NOEXCEPTandSIXTRL_RESTRICT, hopefully improving the performance especially under C++Required Testing:
In order to test this release, please create a new
Settings.cmakefromSettings.cmake.defaultas the changes introduced for PR #123 are also required here and mandate a new Settings format.Any feedback concerning the performance and correctness of this PR would be highly appreciated. In particular, it would be extremly important to verify, that
NS(TriCub)andNS(TriCubData)continue to work as expected. Any changes not already present in the current iterationof PR Enable/disable features, adds TriCub element (disabled by default), string handling in python #123 should be incorporated in this pull-requestNS(SpaceCharge*)elements have been ported directly from the pysixtrack implementation (cf. Feature/sc linedensity interpolation pysixtrack#50). It is very likely that they still contain bugs since we do not currently have any tests verifiying the correctness of the space-charge implementationsNS(TriCub)elements should remain roughly where it is with respect to the current main branch.NS(SpaceCharge*)elements is not set in stone - any feedback would be highly appreciatedRegressions and Bugs:
All tests seem to pass at least on my development machine. However, there is an issue with the OpenCL implementation provided by a recent version of the
Intel OneAPI(cf. issue #131) which is also effecting the currently release version and which should be investigatedTODOs before merging:
In addition to testing and benchmarking (cf. above, thank you again), the remaining tasks are to
NS(SpaceChargeInterpolatedProfile)does work also using a Cuda TrackJobIn Closing, Additional Remarks, and Minor Changes Not Highlighted So Far:
This is a massive change, touching and potentially breaking a lot of differnt parts of the implementation. I am sorry for the sheer volume but I was not very successful in landing these changes in a more digestible fashion. In addition to the changes outlined above (And to what is presented / described in #123), the following additional changes are part of this PR:
NS(Multipole),NS(RFMultipole),NS(BeamBeam4D),NS(BeamBeam6D)) now use a 64Bit integer to store the address rather than having the pointer directly as a datamember. This ensures that storing elements into aCBuffer()on architectures with 32bit pointers produces a binary representation also usable on 64 Bit systems. It also gives additional flexibility concerning the memory region of theNS(Multipole)andNS(RFMultipole)elements to be consistent with their counterparts in the python bindings of SixTrackLib and pysixtracksixtracklib/common/internal/math_constants.h,sixtracklib/common/internal/physics_constants.hwhich provide a templated C++ and a default C implementation for commonly used constants. This is a stepping stone to a type-traits driven version of these files used in a more recent developent version of SixTrackLib, allowing the use of types other thandoubledoublefor floating-point valued quantitiesSIXTRL_UNUSED()macro to properly handle unused / optional method parameters, avoiding warnings and casts using (void) in the method bodydoublevia C++ templatesgoogletest(cf. issue remove underscores from googletest testcase names #124). This PR continues with the attempts to fix all remaining tests which are affectedNOTES: