Making n sub intervals a config class parameter #1090
Conversation
Made n_sub_intervals an attribute for simulationConfigs and updated unit tests to include it.
| @@ -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) | |||
There was a problem hiding this comment.
why can't you just do
self.n_sub_intervals = 101 # we set this to a default 101?
There was a problem hiding this comment.
@matthewholman any particular reason we hide the n_sub_intervals from the configuration file but all the healpix order to be changed?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
There is no reason to hide that from the user, as long as there is a default value.
There was a problem hiding this comment.
what is this parameter so I can add it to the documentation @matthewholman ?
There was a problem hiding this comment.
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.
| @@ -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) | |||
There was a problem hiding this comment.
To keep the same format. I'd make the variable ar_n_sub_intervals
There was a problem hiding this comment.
(edited above to fix my typo)
Renamed n_sub_intervals to ar_n_sub_intervals and set it's default value at the start instead of None. Also added the parameter to the log.
Fixes #1079.
Changed n_sub_intervals in simulation_driver.py to be a config class parameter in simulationConfigs
Review Checklist for Source Code Changes