#2627: Tmunu inherits from ADMBase and StaticConformal without using them
Reporter: Gabriele Bozzola
Status: open
Milestone:
Version:
Type: bug
Priority: trivial
Component:
Comment (by Roland Haas):
Having said that… I can make the full ET compile with minimal modification \(to GRHydro which contained a leftover STORAGE statement for `conformal_state`\) when removing those inherits. It also passes all tests. I will propose them to be declared deprecated in the next release and removed afterwards.
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2627/tmunu-inherits-fr…
#2627: Tmunu inherits from ADMBase and StaticConformal without using them
Reporter: Gabriele Bozzola
Status: open
Milestone:
Version:
Type: bug
Priority: trivial
Component:
Changes (by Roland Haas):
responsible: [] (was )
assignee: Erik Schnetter (was )
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2627/tmunu-inherits-fr…
#2627: Tmunu inherits from ADMBase and StaticConformal without using them
Reporter: Gabriele Bozzola
Status: open
Milestone:
Version:
Type: bug
Priority: trivial
Component:
Changes (by Roland Haas):
status: open (was new)
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2627/tmunu-inherits-fr…
#2627: Tmunu inherits from ADMBase and StaticConformal without using them
Reporter: Gabriele Bozzola
Status: new
Milestone:
Version:
Type: bug
Priority: trivial
Component:
Comment (by Roland Haas):
Historic leftover and backwards compatible thing I would say.
The old Tmunu mechanism \(using the FRIEND mechanism\) had a routine `TmunuBase_SetTmunu` that used `ADMCoupling`'s `STRESSENERGY_guts.h` macros, which may have conceivably used `ADMBase` variables \(at least that is my only guess\). Or it may have been superfluous when the thorn was first created in 2009. The old Tmunu code was removed in git hash [5a412c0](https://bitbucket.org/einsteintoolkit/einsteinbase/commits/5a412c0… "TmunuBase: remove deprecated support for old CalcTmunu mechanism" of [einsteinbase](https://bitbucket.org/einsteintoolkit/einsteinbase).
Right now removing it may be dicey since inheritance is transitive, that is thorns inheriting from `TmunuBase` get access to all the grid functions that `TmunuBase` inherited. There is bound to be a thorn out there that _only_ inherits from `TmunuBase` but still accesses `ADMBase` variables \(valid code in fact\).
This is probably a good candidate for a deprecated features announcement. While I doubt there are Tmunu using thorns that do not also use `ADMBase` there is a case to be made for no longer requiring `StaticConformal` in particular since support for conformaly related metrics is essentially non-existent in current codes.
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2627/tmunu-inherits-fr…
#2627: Tmunu inherits from ADMBase and StaticConformal without using them
Reporter: Gabriele Bozzola
Status: new
Milestone:
Version:
Type: bug
Priority: trivial
Component:
I am not sure if this is a bug or I am missing some second-order effect. The `Tmunu` thorn inherits from `ADMBase` and `StaticConformal` but from what I can tell it doesn’t use anything from either.
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2627/tmunu-inherits-fr…
#2626: Multipole is not OpenMP parallelized
Reporter: Gabriele Bozzola
Status: new
Milestone:
Version:
Type: enhancement
Priority: trivial
Component:
Comment (by Gabriele Bozzola):
I was looking at the inner function `Multipole_Integrate` \(ie, fixed a radius, l, and m\). I think that loops there \(which are over theta and phi\) can be parallelized with no problems. However, you are probably right and the simplest thing to do would be to add pragmas over l and m. The function call is
```
for (int l=0; l <= lmax; l++)
{
for (int m=-l; m <= l; m++)
{
// Integrate sYlm (real + i imag) over the sphere at radius r
Multipole_Integrate(array_size, ntheta,
reY[si][l][m+l], imY[si][l][m+l],
real, imag, th, ph,
&modes(v, i, l, m, 0), &modes(v, i, l, m, 1));
}//loop over m
}//loop over l
```
`Multipole_Integrate` does not touch any input except the last two, so the input can be shared. `modes` is a custom class that contains all the output data and `&modes(v, i, l, m, 0)` is just a pointer to a `CCTK_REAL`. Different iterations are going to have different pointers, so I think that the iterations are all independent without making any change. `l`and `m` are already private, so, probably to parallelize the entire thorn we just need to add
`#pragma omp for collapse(2)`
\(Plus, we’d have to remove an inner pragma in one of the integration methods\)
> You may actually see an extra speedup, if you are using it, by switching from ASCII output to HDF5 output.
Yes, I am already doing that \(which, incidentally, speeds up significantly kuibit as well\).
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2626/multipole-is-not-…
#2626: Multipole is not OpenMP parallelized
Reporter: Gabriele Bozzola
Status: new
Milestone:
Version:
Type: enhancement
Priority: trivial
Component:
Comment (by Roland Haas):
If the loops \(e.g. the ones over radii\) contain calls to the interpolator then they cannot be OpenMP parallelized, since the interpolator \(no Cactus call actually\) is not thread save.
One would have to MPI parallelize but that is not trivial since the interpolator is a MPI collective call \(but different ranks may pass different arguments, so what is needed is “dummy” interpolator calls on those ranks that have run out of radii to work on\). It is also not fully clear if this would speed up \(or not, if the inter-process communication actually dominates\).
The loops over \(l,m\) can be OpenMP parallelized \(no interpolator call there\) can be parallelized using OpenMP \(though watch out for shared temporary arrays and static variables\).
You may actually see an extra speedup, if you are using it, by switching from ASCII output to HDF5 output.
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2626/multipole-is-not-…
#2626: Multipole is not OpenMP parallelized
Reporter: Gabriele Bozzola
Status: new
Milestone:
Version:
Type: enhancement
Priority: trivial
Component:
The `Multipole` thorn is completely serial, while being essentially a series for loops that could be easily parallelized with OpenMP.
\(In one of my typical BBH simulation, `Multipole` is of order of 1-2 % of the execution time.\)
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2626/multipole-is-not-…
#2598: thornyflat - production run segmentation fault
Reporter: Maria
Status: new
Milestone:
Version: ET_2021_11
Type: bug
Priority: major
Component:
Comment (by Anuj Kankani):
Got it, thank you for all the help. Sorry for the mistakes, I don’t have much experience with git/bitbucket.
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2598/thornyflat-produc…
#2598: thornyflat - production run segmentation fault
Reporter: Maria
Status: new
Milestone:
Version: ET_2021_11
Type: bug
Priority: major
Component:
Comment (by Roland Haas):
the way to do it is:
1. fork the simfactory repo on bitbucket
2. create a branch in your fork with the files you want
3. create the pull request in your fork, it will show up in the list of pull requests in the source repository
--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2598/thornyflat-produc…