Hi Chris, Thanks for the changes, I’ve made most of them and pushed them to the repo, as described below. As suggested by Nicolás I’ve set the default resolution in -360 mode to be lower (1080 x 540) so that it fits on the screen in interactive mode. This means its also faster to render.
I also added the -fourpi and -4pi flags as aliases for -360, in support of your proposed naming scheme. Cheers, Daniel > Glad the 360-degree option made it into splash already! > > In making new images with Nico's sims, as well as reproducing 360-degree > plots from some of mine, I came across a few things that I think should be > updated. The first changes affects the polar regions, the second affects the > wrapping around at the phi=+-pi border, the third affects the space reserved > for the colorbar, and the rest are minor. > > > Polar Regions: > > In geometry.f90, subroutine get_coord_limits, case igeom=3, starting at line > 766: > > replace > > if (r > 0. .and. xmin(1) > 0.) then > dphi = atan(rad/r) > xmin(2) = xout(2)-dphi > xmax(2) = xout(2)+dphi > xmin(3) = min(acos((xin(3)-rad)/r),acos((xin(3)+rad)/r)) > xmax(3) = max(acos((xin(3)+rad)/r),acos((xin(3)-rad)/r)) > xmin(3) = max(xmin(3),0.) > xmax(3) = min(xmax(3),pi) > else > > with > > if (r > 0. .and. xmin(1) > 0.) then > rcyl = sqrt(xin(1)**2 + xin(2)**2) > if(rcyl > rad) then > dphi = asin(rad/rcyl) > xmin(2) = xout(2)-dphi > xmax(2) = xout(2)+dphi > else > xmin(2) = -pi > xmax(2) = pi > endif > dtheta = asin(rad/r) > xmin(3) = xout(3)-dtheta > xmax(3) = xout(3)+dtheta > xmin(3) = max(xmin(3),0.) > xmax(3) = min(xmax(3),pi) > else Have pushed these changes - thanks! > The original computation of the polar limits involves acos((z+-rad)/r), which > fails when the argument is greater than 1: the result of acos(argument>1) > yields a NaN, so the irrelevant option is chosen in the min/max statements, > which means the polar pixels have no contributions. I replaced this with > asin(rad/r), which finds the exact boundary of the sphere with radius rad and > center r from the origin. > > I also noticed that the azimuthal selection criteria seemed to possibly miss > a small range of pixels, which got larger as the particle moved away from the > x-y plane. Therefore, I suggest that the phi range depends on > asin(rad/rcyl), where rcyl is the distance to the z-axis. Put another way, > using rcyl projects the sphere onto the x-y plane, and then calculates the > theta limits. > > Note that I don't have anything to enforce the limits for phi to be between > -pi and pi -- this will allow for wrapping around the phi=+-pi border, if you > make the next change. > > > phi=+-pi Border > > In interpolate3D_proj_geom.F90, subroutine get_pixel_limits, starting at line > 582: > > replace > > !if (.not.coord_is_periodic(ixcoord,igeom)) then > if (ipixmin.lt.1) ipixmin = 1 ! make sure they only contribute > if (jpixmin.lt.1) jpixmin = 1 ! to pixels in the image > !endif > !if (.not.coord_is_periodic(iycoord,igeom)) then > if (ipixmax.gt.npixx) ipixmax = npixx ! (note that this optimises > if (jpixmax.gt.npixy) jpixmax = npixy ! much better than using min/max) > !endif > > > with > > if (.not.coord_is_periodic(ixcoord,igeom)) then > if (ipixmin.lt.1) ipixmin = 1 ! make sure they only contribute > if (ipixmax.gt.npixx) ipixmax = npixx ! (note that this optimises > endif > if (.not.coord_is_periodic(iycoord,igeom)) then > if (jpixmin.lt.1) jpixmin = 1 ! to pixels in the image > if (jpixmax.gt.npixy) jpixmax = npixy ! much better than using min/max) > endif > > Removing the pixel limits of -pi and pi for phi allows the wraparound given > that subroutine interpolate3D_proj_geom, lines 255/256, > if (ip < 1) ip = ip + npixx > if (ip > npixx) ip = ip - npixx > puts the pixel limits between 1 and npixx. When I originally did this, I > made the possibility of having two phi regions, one above -pi and one below > pi, but this bit of code, coupled with the get_pixel_limits change, does the > same thing. Of course the tradeoff is that the above two if statements run > on every single ip, and another two if statements run for every single jp, > which is not the case in my original implementation. > Also pushed. > Colorbar Space: > > This is a hack, but add the following to colourbar.f90, subroutine > get_colourbarmargins, line 500: > > barwidth=0. > > This negates any change in yminmargin. Otherwise, the number of pixels in > the y-direction is reduced to leave space for a colorbar, even though there > is no colorbar outside the plot boundary. > > This change is obviously a quick fix; it would be better to add another > option to this subroutine since this hack will no doubt screw up plots where > the colorbar is outside the plotting range. I didn't try to figure that out > since I didn't want to screw something else up for non-360 plots. Happy to discuss this, seems bad to do this by default. > Minor suggestions: > > Here are some other, less-involved things I noticed: > > 1. In options_page.f90, lines 303 and 304: > print*,'18) 4320 x 2160 pixels (360s)' > print*,'19) 8640 x 4320 pixels (720s)' > should be > print*,'18) 4320 x 2160 pixels (2160s)' > print*,'19) 8640 x 4320 pixels (4320s)' > since the number in front of the 's' refers to the number of polar pixels (at > least that's the convention YouTube uses). > Now fixed. > 2. Also in options_page.f90, at lines 394 to 492, it appears that the case > numbers are off by 1 according to the lines I copied above: > case(18) > papersizex = 27320. > papersizey = 3072. > case(19) > papersizex = 4320. > papersizey = 2160. > case(20) > papersizex = 8640. > papersizey = 4320. > Here Case 19 is 2160s, but above it is case 18. I'm not sure about this > being wrong, though, since Nico's plots, which used the default -360 option > only, were 2160s. Sorry if there isn't actually anything wrong with this > portion. Now fixed. > 3. When viewed in 360-degree mode, the legend at the top of the plot and the > color bar at the bottom will be severely distorted, perhaps even unreadable. > I suggest putting these items at least 1/3 of the way from the top/bottom of > the plot. They will still be out of the way of the center of the image, > which is usually the most important view, but they will not be too distorted > to read. (I also prefer a vertical colorbar as most of my stars in the GC > sims orbit left to right, but that's just a matter of > personal/simulation-dependent preference. I imagine disk simulations would > also benefit from a vertical colorbar since I imagine there are not vertical > structures that the colorbar could completely block .) Have changed the default style to be vertical in 360 mode > > 4. I also like it better with "nomenu = .true." in splash.f90 commented out. > This way I can easily adjust plot limits, log/linear, etc. But this is also > personal preference, and so you might want to leave the default case the way > it is. I’ve left this as default. The main issue is that it’s not obvious to the novice user that one should select theta-phi as the axes. With the reduced default resolution it is hopefully now easier to adjust things interactively anyway. > Please let me know if you have any questions about the suggestions I made. > Hopefully the main suggestions I made (polar region and phi=+-pi border) will > not negatively affect any other aspect of Splash. (I also hope that my > suggestions are indeed correct; I did test them pretty thoroughly, but I'll > do this more as I continue to make more 360-degree plots with this new tool.) Happy to discuss more next week :) -- You received this message because you are subscribed to the Google Groups "SPLASH users forum" group. To unsubscribe from this group and stop receiving emails from it, send an email to splash-users+unsubscr...@googlegroups.com. To post to this group, send email to splash-users@googlegroups.com. Visit this group at https://groups.google.com/group/splash-users. For more options, visit https://groups.google.com/d/optout.
