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.

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

Making n sub intervals a config class parameter #1090

Merged
merged 3 commits into from
Jan 11, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 3 additions & 1 deletion src/sorcha/ephemeris/simulation_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ def create_ephemeris(orbits_df, pointings_df, args, sconfigs):
nside : integer
The nside value used for the HEALPIx calculations. Must be a
power of 2 (1, 2, 4, ...) nside=64 is current default.
n_sub_intervals: int
Number of sub-intervals for the Lagrange interpolation (default: 101)

Returns
-------
Expand Down Expand Up @@ -109,7 +111,7 @@ def create_ephemeris(orbits_df, pointings_df, args, sconfigs):
picket_interval = sconfigs.simulation.ar_picket
obsCode = sconfigs.simulation.ar_obs_code
nside = 2**sconfigs.simulation.ar_healpix_order
n_sub_intervals = 101 # configs["n_sub_intervals"]
n_sub_intervals = sconfigs.simulation.n_sub_intervals

ephemeris_csv_filename = None
if args.output_ephemeris_file and args.outpath:
Expand Down
37 changes: 37 additions & 0 deletions src/sorcha/utilities/sorchaConfigs.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ class simulationConfigs:
ar_healpix_order: int = None
"""the order of healpix which we will use for the healpy portions of the code."""

n_sub_intervals: int = None
"""Number of sub-intervals for the Lagrange ephemerides interpolation (default: 101)"""

_ephemerides_type: str = None
"""Simulation used for ephemeris input."""

Expand Down Expand Up @@ -111,6 +114,7 @@ def _validate_simulation_configs(self):
self.ar_fov_buffer = cast_as_float(self.ar_fov_buffer, "ar_fov_buffer")
self.ar_picket = cast_as_int(self.ar_picket, "ar_picket")
self.ar_healpix_order = cast_as_int(self.ar_healpix_order, "ar_healpix_order")
self.n_sub_intervals = cast_as_int_or_set_default(self.n_sub_intervals, "n_sub_intervals", 101)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why can't you just do
self.n_sub_intervals = 101 # we set this to a default 101?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@matthewholman any particular reason we hide the n_sub_intervals from the configuration file but all the healpix order to be changed?

Copy link
Collaborator

@mschwamb mschwamb Jan 10, 2025

Choose a reason for hiding this comment

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

With the latest updates we can expose to the user which is how @Little-Ryugu is implementing it but it would default to 101? If we keep it this way, @Little-Ryugu I don't think you need a new function is there a way to use cast_as_int or check if it's none (like the trailing loss flag which is only sometimes used) and then set the value to the default?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no reason to hide that from the user, as long as there is a default value.

Copy link
Collaborator

@mschwamb mschwamb Jan 10, 2025

Choose a reason for hiding this comment

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

what is this parameter so I can add it to the documentation @matthewholman ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was afraid you would ask. That is the number of sub-intervals used in evaluating a Lagrange interpolation function to make sure fast-movers are not missed. It's pretty arbitrary.

Copy link
Collaborator

@mschwamb mschwamb Jan 10, 2025

Choose a reason for hiding this comment

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

To keep the same format. I'd make the variable ar_n_sub_intervals

Copy link
Collaborator

@mschwamb mschwamb Jan 10, 2025

Choose a reason for hiding this comment

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

(edited above to fix my typo)

elif self._ephemerides_type == "external":
# makes sure when these are not needed that they are not populated
check_key_doesnt_exist(self.ar_ang_fov, "ar_ang_fov", "but ephemerides type is external")
Expand Down Expand Up @@ -1307,6 +1311,39 @@ def cast_as_bool_or_set_default(value, key, default):
return default


def cast_as_int_or_set_default(value, key, default):

# replaces PPGetBoolOrExit: checks to make sure the value can be cast as a bool.
"""
Checks to see if value can be cast as an int and if not set (equals None) gives default int.

Parameters
-----------
value : object attribute
value of the config file attribute

key : string
The key being checked.

default : int
default int if value is None

Returns
----------
value as an int
"""

if value is not None:
try:
int(value)
except ValueError:
logging.error(f"ERROR: expected an int for config parameter {key}. Check value in config file.")
sys.exit(f"ERROR: expected an int for config parameter {key}. Check value in config file.")
return value
elif value is None:
return default


def PrintConfigsToLog(sconfigs, cmd_args):
"""
Prints all the values from the config file and command line to the log.
Expand Down
3 changes: 2 additions & 1 deletion tests/sorcha/test_sorchaConfigs.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"ar_picket": 1,
"ar_obs_code": "X05",
"ar_healpix_order": 6,
"n_sub_intervals": 101
}

correct_filters_read = {"observing_filters": "r,g,i,z,u,y", "survey_name": "rubin_sim"}
Expand Down Expand Up @@ -261,7 +262,7 @@ def test_simulationConfigs_float(key_name):
)


@pytest.mark.parametrize("key_name", ["ar_picket", "ar_healpix_order"])
@pytest.mark.parametrize("key_name", ["ar_picket", "ar_healpix_order","n_sub_intervals"])
def test_simulationConfigs_int(key_name):
"""
Tests that wrong inputs for simulationConfigs int attributes is caught correctly
Expand Down
Loading