Modify

Opened 8 years ago

Last modified 7 years ago

#429 reopened enhancement

Parallelising AEILocalInterp

Reported by: Erik Schnetter Owned by: Erik Schnetter
Priority: major Milestone:
Component: EinsteinToolkit thorn Version:
Keywords: Cc:

Description

The enclosed patch parallelises AEILocalInterp via OpenMP.

This leads to a slight change in behaviour. Currently, AEILocalInterp traverses the list of points sequentially, and aborts when the first error is encountered. After parallelisation, there is no fixed order in which the points are traversed, and if several errors are encountered, any one of the errors may be returned, not necessarily the first. I am not aware of any thorn that would or should rely on such an ordering.

This patch also adds "restrict" and "const" statements that may improve performance as it gives the compiler more information about dependencies between pointers.

Attachments (1)

AEILocalInterp-openmp.diff (66.9 KB) - added by Erik Schnetter 8 years ago.

Download all attachments as: .zip

Change History (6)

Changed 8 years ago by Erik Schnetter

Attachment: AEILocalInterp-openmp.diff added

comment:1 Changed 8 years ago by Erik Schnetter

Status: newreview

comment:2 Changed 7 years ago by Erik Schnetter

*ping*

comment:3 Changed 7 years ago by Frank Löffler

Owner: set to Erik Schnetter
Status: reviewassigned

The patch mostly looks good, but I noticed two things:

  • next time please try to prepare two different patches: the one with restrict&const can probably be applied without discussion, and the remaining changes are then not that long
  • there are some #if 0 close to the new openmp statements. It would be nice to either remove them (since they are not used in any case), or if they should stay at least for some time, please use a #define to dis- or enable all at the same time, and to have some more information in the source. It reads better to have #if DISABLE_OPENMP (or similar) than #if 0

Frank

comment:4 Changed 7 years ago by Erik Schnetter

Resolution: fixed
Status: assignedclosed

I removed the #if 0 parts of the code.

Applied.

comment:5 Changed 7 years ago by Erik Schnetter

Resolution: fixed
Status: closedreopened

I reverted this patch because it led to test suite failures in SphericalHarmonicRecon (reported in #873). Another problem with this patch is that it (apparently) leads to too much output, since interpolation is not aborted any more after the first error.

We will revisit this after the ET_2011_05 release.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as reopened The owner will remain Erik Schnetter.
Next status will be 'review'.
as The resolution will be set.
to The owner will be changed from Erik Schnetter to the specified user.
The owner will be changed from Erik Schnetter to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.