Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

editing defaults for consistency #88

Merged
merged 2 commits into from
Dec 16, 2024
Merged

editing defaults for consistency #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
085f662
editing defaults for consistency
CarrieFilion Oct 10, 2024
283f9d8
percolating some changes through -
CarrieFilion Oct 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion expui/BiorthBasis.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,7 @@ namespace BasisClasses
nmax = 18;
mmax = 6;
mlim = std::numeric_limits<int>::max();
lmaxfid = 128;
lmaxfid = 72;
nmaxfid = 64;
ncylnx = 256;
ncylny = 128;
Expand Down
2 changes: 1 addition & 1 deletion src/expand.H
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
KEYWORD EXPLANATION DEFAULT
---------- ---------------------------- --------
lmax = maximum harmonic order 4
nmax = maxinum radial order 10
nmax = maximum radial order 18
nlog = interval between log output 10
nlist = interval between p-s output 50
nsteps = total number of steps to run 500
Expand Down
4 changes: 2 additions & 2 deletions utils/ICs/cylcache.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ main(int ac, char **av)
("ctype", "DiskHalo radial coordinate scaling type (one of: Linear, Log, Rat)",
cxxopts::value<string>(ctype)->default_value("Log"))
("LMAXFID", "Maximum angular order for spherical basis in adaptive construction of the cylindrical basis",
cxxopts::value<int>(LMAXFID)->default_value("32"))
cxxopts::value<int>(LMAXFID)->default_value("72"))
("NMAXFID", "Maximum radial order for the spherical basis in adapative construction of the cylindrical basis",
cxxopts::value<int>(NMAXFID)->default_value("32"))
("MMAX", "Maximum azimuthal order for the cylindrical basis",
Expand All @@ -341,7 +341,7 @@ main(int ac, char **av)
("NCYLODD", "Number of vertically odd basis functions per harmonic order",
cxxopts::value<int>(NCYLODD)->default_value("6"))
("NMAX", "Total number of basis functions per harmonic order",
cxxopts::value<int>(NMAX)->default_value("20"))
cxxopts::value<int>(NMAX)->default_value("18"))
("VFLAG", "Diagnostic flag for EmpCylSL",
cxxopts::value<int>(VFLAG)->default_value("31"))
("expcond", "Use analytic target density rather than particle distribution",
Expand Down
8 changes: 4 additions & 4 deletions utils/ICs/initial.cc
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lmaxfid=128 will be really computationally expensive -- I wonder if we want to go the other way and change the defaults elsewhere to a lower value?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, lmaxfid=128 is a lot. Do you have a suggestion? 72 or 96 maybe?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the past I found diminishing returns after 72 unless really going for it on the other parameters (it takes a lot of integration points to resolve even 96!). Unless we want this to be super-production-ready code, I'd set at 72.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's do 72.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lmaxfid is set to 128 in src/cylinder.cc and expui/BiorthBasis.H - I'll change it to 72 throughout.

Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,9 @@ main(int ac, char **av)
("LMAX", "Harmonic order for halo expansion",
cxxopts::value<int>(LMAX)->default_value("18"))
("LMAXFID", "Harmonic order for EOF spherical model",
cxxopts::value<int>(LMAXFID)->default_value("48"))
cxxopts::value<int>(LMAXFID)->default_value("72"))
("NMAXFID", "Radial order for EOF spherical model",
cxxopts::value<int>(NMAXFID)->default_value("48"))
cxxopts::value<int>(NMAXFID)->default_value("64"))
("MMAX", "Aximuthal order for Cylindrical expansion",
cxxopts::value<int>(MMAX)->default_value("6"))
("NUMX", "Number of knots in radial dimension of meridional grid",
Expand Down Expand Up @@ -434,7 +434,7 @@ main(int ac, char **av)
("NOUT", "Number of radial terms in diagnostic basis file for cylinder",
cxxopts::value<int>(NOUT)->default_value("18"))
("NODD", "Number of vertically odd terms in cylindrical expansion",
cxxopts::value<int>(NODD)->default_value("6"))
cxxopts::value<int>(NODD)->default_value("9"))
("NMAXH", "Number of radial terms for spherical expansion",
cxxopts::value<int>(NMAXH)->default_value("18"))
("NMAXD", "Number of radial terms for cylindrical expansion",
Expand Down Expand Up @@ -518,7 +518,7 @@ main(int ac, char **av)
("SCSPH", "Scale for Spherical SL coordinate mapping",
cxxopts::value<double>(SCSPH)->default_value("1.0"))
("RSPHSL", "Maximum halo expansion radius",
cxxopts::value<double>(RSPHSL)->default_value("47.5"))
cxxopts::value<double>(RSPHSL)->default_value("1.95"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another strategy here would be to accept whatever the maximum radius in the input table is. That is probably more user-error proof?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason for RSPHSL (which could have a better name?) is to guide SLGridSph for tabled models whose density gets close to machine zero, which will break the SL solver. I suppose that choice could be selected from the table itself automatically with some heuristic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I guess my hope is that users will have good density models, especially if they are making a set of ICs. But I guess this is a way out if a user had a table they couldn't change (for some weird reason).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. I'm okay with forcing the user to change their table to something consistent with SL. I'm in favor of reducing the number of free parameters generally. So I'm in favor of your first proposal: get rid of RSPHSL and use the rmax value from the table.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to implement this suggestion but it probably is the best fix

("ASCALE", "Radial scale length for disk basis construction",
cxxopts::value<double>(ASCALE)->default_value("1.0"))
("ASHIFT", "Fraction of scale length for shift in conditioning function",
Expand Down
2 changes: 1 addition & 1 deletion utils/SL/diskpot.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ main(int argc, char** argv)
int cmap = 0;
double scale = 1.0;

int Lmax=16, Nmax=10;
int Lmax=16, Nmax=18;
int numr=10000;
double rmin=0.0001, rmax=1.0;
double delr=0.01, xmax=1.0, zmax=0.1;
Expand Down
2 changes: 1 addition & 1 deletion utils/SL/oftest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ int main(int argc, char** argv)
("N,mc", "number of particles for Monte Carlo realization",
cxxopts::value<int>(N)->default_value("10000"))
("nmax", "maximum number of radial harmonics in the expansion",
cxxopts::value<int>(nmax)->default_value("10"))
cxxopts::value<int>(nmax)->default_value("18"))
("r,rmin", "minimum radius for the SL grid",
cxxopts::value<double>(rmin)->default_value("0.0001"))
("R,rmax", "maximum radius for the SL grid",
Expand Down
2 changes: 1 addition & 1 deletion utils/SL/qtest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ int main(int argc, char** argv)
("Lmax", "maximum number of angular harmonics in the expansion",
cxxopts::value<int>(Lmax)->default_value("2"))
("nmax", "maximum number of radial harmonics in the expansion",
cxxopts::value<int>(nmax)->default_value("10"))
cxxopts::value<int>(nmax)->default_value("18"))
("numr", "radial knots for the SL grid",
cxxopts::value<int>(numr)->default_value("1000"))
("rmin", "minimum radius for the SL grid",
Expand Down
2 changes: 1 addition & 1 deletion utils/SL/slabchk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ main(int argc, char** argv)
("K,kmax", "maximum order of in-plane harmonics",
cxxopts::value<int>(kmax)->default_value("4"))
("N,nmax", "maximum number of vertical harmonics",
cxxopts::value<int>(nmax)->default_value("10"))
cxxopts::value<int>(nmax)->default_value("18"))
("n,numz", "size of vertical grid",
cxxopts::value<int>(numz)->default_value("1000"))
("Z,zmax", "maximum extent of vertical grid",
Expand Down
2 changes: 1 addition & 1 deletion utils/SL/slshift.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ main(int argc, char** argv)
("Lmax", "maximum number of angular harmonics in the expansion",
cxxopts::value<int>(Lmax)->default_value("2"))
("nmax", "maximum number of radial harmonics in the expansion",
cxxopts::value<int>(nmax)->default_value("10"))
cxxopts::value<int>(nmax)->default_value("18"))
("numr", "radial knots in the shift operator",
cxxopts::value<int>(numr)->default_value("1000"))
("rmin", "minimum radius for the shift operator",
Expand Down