[Einstein Toolkit] #1415: GRHydro updates

List overview All Threads
Download

newer

older

[Einstein Toolkit] #605:...

[Einstein Toolkit] #1422:...

Einstein Toolkit

4 Aug 2013 4 Aug '13

2:31 a.m.

#1415: GRHydro updates -----------------------------------+---------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: new Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: development version Keywords: GRHydro | -----------------------------------+---------------------------------------- accumulated GRHydro changes since before the CGWAS school (July 22nd).

Includes the PPM changes from the workshop. Does not yet include the staggered vector potential work since this seems to be still in early stages.

Will commit after Thursday unless objections are raised.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Show replies by date

Einstein Toolkit

4 Aug 4 Aug

2:32 a.m.

#1415: GRHydro updates ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: GRHydro ------------------------------------+--------------------------------------- Changes (by rhaas):

* status: new => review

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:1 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

5 Aug 5 Aug

3:45 p.m.

Comment (by knarf):

Partial review: 0001 looks good, although intendation looks a bit odd in places (e.g. see calculation of beta1).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:2 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

3:47 p.m.

Comment (by knarf):

Why is 0002 necessary? I don't like parameters with a name *_hack, for obvious reasons.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:3 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

4:29 p.m.

Comment (by rhaas):

Replying to [comment:3 knarf]:

...

Why is 0002 necessary? I don't like parameters with a name *_hack, for

obvious reasons. There is already an octant_hack :-) . The option prevents some aborts in prim2con which aborts for hot eos if some EOS calls failed. Essentially it affects points in the symmetry region that will be filled in by the symmetry operation later on. Affected points could be eg on the coarse grid and only be filled in by the symmetry call after restriction.

The "correct" fix to this seems to me to have a prim2confailed mask similar to con2primfailed (or re-used that mask) and only abort if the mask is set after restriction.

Until then the _hack parameters provide a workaround that lets on run some runs that otherwise fail.

I am not wedded to this parameter though (mostly I just like git and svn to be in sync as much as possible).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:4 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

4:31 p.m.

Comment (by rhaas):

Replying to [comment:2 knarf]:

...

Partial review: 0001 looks good, although intendation looks a bit odd in

places (e.g. see calculation of beta1). Thank you for the review. Indentation in this file seems to vary between 1 space and 3 spaces. Within each block it is consistent though (though the text in the comments does not align with the code admittedly). I'll commit as is with the other once they are reviewed (one way or the other).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:5 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

5:11 p.m.

Comment (by knarf):

Replying to [comment:4 rhaas]:

...

There is already an octant_hack :-) . The option prevents some aborts in

prim2con which aborts for hot eos if some EOS calls failed. Essentially it affects points in the symmetry region that will be filled in by the symmetry operation later on.

Why don't we not first fill in the points with the correct primitives and do prim2con then? If it still fails then, it should not only fail in the symmetry zones, but also outside, as the symmetry zones should be exact copies of the values outside, and prim2con is a local operation.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:6 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

5:13 p.m.

Comment (by knarf):

0003 depends on 0002 and might not be necessary, 0004 looks good.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:7 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

5:25 p.m.

Comment (by knarf):

0005 is fine.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:8 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

6 Aug 6 Aug

8:48 a.m.

Comment (by knarf):

0006 is fine too.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:9 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

12 Aug 12 Aug

9:16 p.m.

Comment (by knarf):

0007 should call GRHydro_RefinementLevel within the scheduler instead. No need to repeat this expression.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:10 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:20 p.m.

Comment (by knarf):

0008: Any reason why the keyerr couldn't be allocated by Cactus instead? I would prefer that. Also, if you really must allocate it there, use cctk_ash[]. Also: please remove the comment that talks about that malloc would be better when you now use malloc.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:11 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:21 p.m.

Comment (by knarf):

0009 should be fine, assuming the interface is correct :)

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:12 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:21 p.m.

Comment (by knarf):

Please apply 0010

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:13 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:25 p.m.

Comment (by knarf):

Why does 0011 save time?

Only cosmetics: why two nested loops for evolve_mhd and transport_constraints?

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:14 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:26 p.m.

Comment (by knarf):

0012: If this disables a mistake, why isn't the mistake then not taken out of the code entirely, instead of only protecting it using the cpp?

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:15 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:27 p.m.

Comment (by knarf):

Please apply 0013 (yeah!)

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:16 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:29 p.m.

Comment (by knarf):

Also please apply 0014 to 0017.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:17 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:30 p.m.

Comment (by knarf):

0018: It would be nice to decide when this code should be completely removed. Just commenting it messes up the source and shouldn't be done if the plan is to dump this entirely in the future.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:18 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:31 p.m.

Comment (by knarf):

0019 should be fine.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:19 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

13 Aug 13 Aug

9:57 a.m.

#1415: GRHydro updates ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: closed Priority: minor | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: fixed | Keywords: GRHydro ------------------------------------+--------------------------------------- Changes (by rhaas):

* status: review => closed * resolution: => fixed

Comment:

Committed 0001, 0004, 0005, 0006, 0007 (renamed GRHydro_reflvel to reflevel, I will not schedule the scheduled routine since I do not know from where con2prim will be called and would have to make very sure that the extra GRHydro_RefinementLevel runs just before, for just a diagnostic output this seems not worthwhole. Much more sensible in that case to simple call Carpet's GetRefinementLevel alias routine), applied 0008 using ash instead of lsh (making this a grid function is awkward since it is only used for one combination of evolution_method, evolve_mhd, evolve_temperature and recon_vars some of which are tested in Fortran code and not in schedule.ccl, 0009, 0010, 0011 removed the nested ifs, re- worded commit message, 0012 removing lines of code, 0013, 0014 to 0017. These are revs 563 - 579 of GRHydro.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:20 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

10:03 a.m.

Comment (by rhaas):

Applied 00189 as is. This code is under heavy development. I do not want to create huge conflicts with the development code by taking out pieces of code at random.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1415#comment:21 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

4804

Age (days ago)

4813

Last active (days ago)

trac@lists.einsteintoolkit.org

21 comments

1 participants

tags (0)

participants (1)

Einstein Toolkit