From 538a0ddb0232e6d710c588796dc36cd61c4241d5 Mon Sep 17 00:00:00 2001 From: Ryan Lyttle Date: Thu, 9 Jan 2025 14:06:02 +0000 Subject: [PATCH 1/9] Changed mag limit cut to use trailed source mag --- src/sorcha/modules/PPMagnitudeLimit.py | 7 +++++-- tests/sorcha/test_PPMagnitudeLimit.py | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/sorcha/modules/PPMagnitudeLimit.py b/src/sorcha/modules/PPMagnitudeLimit.py index e34baadd..20506cca 100644 --- a/src/sorcha/modules/PPMagnitudeLimit.py +++ b/src/sorcha/modules/PPMagnitudeLimit.py @@ -1,4 +1,4 @@ -def PPMagnitudeLimit(observations, mag_limit): +def PPMagnitudeLimit(observations, mag_limit, colname="trailedSourceMag"): """ Filter that performs a straight cut on apparent PSF magnitude based on a defined threshold. @@ -11,6 +11,9 @@ def PPMagnitudeLimit(observations, mag_limit): mag_limit : float Limit for apparent magnitude cut. + colname : string, optional + Column name to apply the magnitude cut. + Default = "TrailedSourceMag" Returns ----------- observations : pandas dataframe @@ -19,7 +22,7 @@ def PPMagnitudeLimit(observations, mag_limit): """ - observations = observations[observations["PSFMag"] < mag_limit] + observations = observations[observations[colname] < mag_limit] observations.reset_index(drop=True, inplace=True) return observations diff --git a/tests/sorcha/test_PPMagnitudeLimit.py b/tests/sorcha/test_PPMagnitudeLimit.py index 3929745e..ad13d189 100644 --- a/tests/sorcha/test_PPMagnitudeLimit.py +++ b/tests/sorcha/test_PPMagnitudeLimit.py @@ -6,10 +6,10 @@ def test_PPMagnitudeLimit(): from sorcha.modules.PPMagnitudeLimit import PPMagnitudeLimit - test_input = pd.DataFrame({"PSFMag": np.arange(15, 25)}) + test_input = pd.DataFrame({"trailedSourceMag": np.arange(15, 25)}) test_output = PPMagnitudeLimit(test_input, 18.0) - assert_equal(test_output["PSFMag"].values, [15, 16, 17]) + assert_equal(test_output["trailedSourceMag"].values, [15, 16, 17]) test_zero = PPMagnitudeLimit(test_input, 14.0) assert len(test_zero) == 0 From 184db3f0017fbdc3ea351bd71ba0b90a434406f7 Mon Sep 17 00:00:00 2001 From: Ryan Lyttle Date: Thu, 9 Jan 2025 14:13:40 +0000 Subject: [PATCH 2/9] Update PPMagnitudeLimit.py --- src/sorcha/modules/PPMagnitudeLimit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sorcha/modules/PPMagnitudeLimit.py b/src/sorcha/modules/PPMagnitudeLimit.py index 20506cca..62de56cd 100644 --- a/src/sorcha/modules/PPMagnitudeLimit.py +++ b/src/sorcha/modules/PPMagnitudeLimit.py @@ -12,7 +12,7 @@ def PPMagnitudeLimit(observations, mag_limit, colname="trailedSourceMag"): Limit for apparent magnitude cut. colname : string, optional - Column name to apply the magnitude cut. + Column thats used to apply the magnitude cut. Default = "TrailedSourceMag" Returns ----------- From 75c953b4c00e51f7b76aff7525add350f538cc32 Mon Sep 17 00:00:00 2001 From: Ryan Lyttle Date: Thu, 9 Jan 2025 15:41:20 +0000 Subject: [PATCH 3/9] Fixed bug with sorchaConfigs Made sure that wrong inputs for SNR_limit and mag_limit are caught correctly. --- src/sorcha/utilities/sorchaConfigs.py | 2 ++ tests/sorcha/test_sorchaConfigs.py | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/sorcha/utilities/sorchaConfigs.py b/src/sorcha/utilities/sorchaConfigs.py index 4eacb458..545beaf5 100644 --- a/src/sorcha/utilities/sorchaConfigs.py +++ b/src/sorcha/utilities/sorchaConfigs.py @@ -750,6 +750,7 @@ def _validate_expert_configs(self): None """ if self.SNR_limit is not None: + self.SNR_limit = cast_as_float(self.SNR_limit, "SNR_limit") self.SNR_limit_on = True if self.SNR_limit < 0: logging.error("ERROR: SNR limit is negative.") @@ -758,6 +759,7 @@ def _validate_expert_configs(self): self.SNR_limit_on = False if self.mag_limit is not None: + self.mag_limit = cast_as_float(self.mag_limit, "mag_limit") self.mag_limit_on = True if self.mag_limit < 0: logging.error("ERROR: magnitude limit is negative.") diff --git a/tests/sorcha/test_sorchaConfigs.py b/tests/sorcha/test_sorchaConfigs.py index 6f422a06..831ab9bf 100644 --- a/tests/sorcha/test_sorchaConfigs.py +++ b/tests/sorcha/test_sorchaConfigs.py @@ -963,6 +963,19 @@ def test_activity_config(): # expert config test +@pytest.mark.parametrize("key_name", ["SNR_limit", "mag_limit"]) +def test_expert_config_float(key_name): + """ + tests that wrong inputs for expertConfigs float attributes is caught correctly + """ + + expect_configs = correct_expert.copy() + expect_configs[key_name] = "str" + with pytest.raises(SystemExit) as error_text: + test_configs = expertConfigs(**expect_configs) + + assert error_text.value.code == f"ERROR: expected a float for config parameter {key_name}. Check value in config file." + @pytest.mark.parametrize("key_name, error_name", [("SNR_limit", "SNR"), ("mag_limit", "magnitude")]) def test_expert_config_bounds(key_name, error_name): From 486abfd3ad9893f88b43f06d328e6db837477924 Mon Sep 17 00:00:00 2001 From: Ryan Lyttle Date: Thu, 9 Jan 2025 16:48:30 +0000 Subject: [PATCH 4/9] fixed bug in sorchaConfigs Changed position_decimals and magnitude_decimals to be cast as integers instaec of floats. --- src/sorcha/utilities/sorchaConfigs.py | 8 ++++---- tests/sorcha/test_sorchaConfigs.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sorcha/utilities/sorchaConfigs.py b/src/sorcha/utilities/sorchaConfigs.py index 545beaf5..85f7e4ce 100644 --- a/src/sorcha/utilities/sorchaConfigs.py +++ b/src/sorcha/utilities/sorchaConfigs.py @@ -580,10 +580,10 @@ class outputConfigs: output_columns: str = None """Controls which columns are in the output files.""" - position_decimals: float = None + position_decimals: int = None """position decimal places""" - magnitude_decimals: float = None + magnitude_decimals: int = None """magnitude decimal places""" def __post_init__(self): @@ -628,9 +628,9 @@ def _validate_decimals(self): None """ if self.position_decimals is not None: - self.position_decimals = cast_as_float(self.position_decimals, "position_decimals") + self.position_decimals = cast_as_int(self.position_decimals, "position_decimals") if self.magnitude_decimals is not None: - self.magnitude_decimals = cast_as_float(self.magnitude_decimals, "magnitude_decimals") + self.magnitude_decimals = cast_as_int(self.magnitude_decimals, "magnitude_decimals") if self.position_decimals is not None and self.position_decimals < 0: logging.error("ERROR: decimal places config variables cannot be negative.") sys.exit("ERROR: decimal places config variables cannot be negative.") diff --git a/tests/sorcha/test_sorchaConfigs.py b/tests/sorcha/test_sorchaConfigs.py index 831ab9bf..8543de5d 100644 --- a/tests/sorcha/test_sorchaConfigs.py +++ b/tests/sorcha/test_sorchaConfigs.py @@ -883,7 +883,7 @@ def test_outputConfigs_inlist(key_name, expected_list): @pytest.mark.parametrize("key_name", ["position_decimals", "magnitude_decimals"]) def test_outputConfigs_decimel_check(key_name): """ - Checks that if deciamels are not float or are negative it is caught correctly + Checks that if deciamels are not int or are negative it is caught correctly """ correct_output = { "output_format": "csv", @@ -900,7 +900,7 @@ def test_outputConfigs_decimel_check(key_name): test_configs = outputConfigs(**output_configs) assert ( error_text.value.code - == f"ERROR: expected a float for config parameter {key_name}. Check value in config file." + == f"ERROR: expected an int for config parameter {key_name}. Check value in config file." ) correct_output = { "output_format": "csv", From ac254015ea5e31533328d33b45328204dc3554ac Mon Sep 17 00:00:00 2001 From: Ryan Lyttle Date: Thu, 9 Jan 2025 16:57:25 +0000 Subject: [PATCH 5/9] Update test_PrintConfigsToLog.txt Updated for unit test --- tests/data/test_PrintConfigsToLog.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/data/test_PrintConfigsToLog.txt b/tests/data/test_PrintConfigsToLog.txt index 94d3901b..49f5cf28 100644 --- a/tests/data/test_PrintConfigsToLog.txt +++ b/tests/data/test_PrintConfigsToLog.txt @@ -227,6 +227,6 @@ sorcha.utilities.sorchaConfigs INFO ...the healpix order is: 6 sorcha.utilities.sorchaConfigs INFO No lightcurve model is being applied. sorcha.utilities.sorchaConfigs INFO Output files will be saved in path: ./ with filestem testout sorcha.utilities.sorchaConfigs INFO Output files will be saved as format: csv -sorcha.utilities.sorchaConfigs INFO In the output, positions will be rounded to 7.0 decimal places. -sorcha.utilities.sorchaConfigs INFO In the output, magnitudes will be rounded to 3.0 decimal places. +sorcha.utilities.sorchaConfigs INFO In the output, positions will be rounded to 7 decimal places. +sorcha.utilities.sorchaConfigs INFO In the output, magnitudes will be rounded to 3 decimal places. sorcha.utilities.sorchaConfigs INFO The output columns are set to: basic From d79284d092d742d825040615b5796da1de08e5c3 Mon Sep 17 00:00:00 2001 From: Ryan Lyttle Date: Thu, 9 Jan 2025 17:18:56 +0000 Subject: [PATCH 6/9] made class attributes lower case made SNR_limit, SNR_limit_on and default_SNR_cut all lower case so they can be properly read in from the config file. --- src/sorcha/modules/PPRandomizeMeasurements.py | 2 +- src/sorcha/sorcha.py | 6 ++--- src/sorcha/utilities/sorchaConfigs.py | 26 +++++++++---------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/sorcha/modules/PPRandomizeMeasurements.py b/src/sorcha/modules/PPRandomizeMeasurements.py index b09ddd37..c26c73ff 100644 --- a/src/sorcha/modules/PPRandomizeMeasurements.py +++ b/src/sorcha/modules/PPRandomizeMeasurements.py @@ -70,7 +70,7 @@ def randomizeAstrometryAndPhotometry(observations, sconfigs, module_rngs, verbos # default SNR cut can be disabled in the config file under EXPERT # at low SNR, high photometric sigma causes randomisation to sometimes # grossly inflate/decrease magnitudes. - if sconfigs.expert.default_SNR_cut: + if sconfigs.expert.default_snr_cut: verboselog("Removing all observations with SNR < 2.0...") observations = PPSNRLimit(observations.copy(), 2.0) diff --git a/src/sorcha/sorcha.py b/src/sorcha/sorcha.py index 3591047b..1ba70eb4 100755 --- a/src/sorcha/sorcha.py +++ b/src/sorcha/sorcha.py @@ -276,14 +276,14 @@ def runLSSTSimulation(args, sconfigs): ) verboselog("Number of rows AFTER applying FOV filters: " + str(len(observations.index))) - if sconfigs.expert.SNR_limit_on and len(observations.index) > 0: + if sconfigs.expert.snr_limit_on and len(observations.index) > 0: verboselog( "Dropping observations with signal to noise ratio less than {}...".format( - sconfigs.expert.SNR_limit + sconfigs.expert.snr_limit ) ) verboselog("Number of rows BEFORE applying SNR limit filter: " + str(len(observations.index))) - observations = PPSNRLimit(observations, sconfigs.expert.SNR_limit) + observations = PPSNRLimit(observations, sconfigs.expert.snr_limit) verboselog("Number of rows AFTER applying SNR limit filter: " + str(len(observations.index))) if sconfigs.expert.mag_limit_on and len(observations.index) > 0: diff --git a/src/sorcha/utilities/sorchaConfigs.py b/src/sorcha/utilities/sorchaConfigs.py index 85f7e4ce..8d37dc99 100644 --- a/src/sorcha/utilities/sorchaConfigs.py +++ b/src/sorcha/utilities/sorchaConfigs.py @@ -709,10 +709,10 @@ def _validate_activity_configs(self): class expertConfigs: """Data class for holding expert section configuration file keys and validating them.""" - SNR_limit: float = None + snr_limit: float = None """Drops observations with signal to noise ratio less than limit given""" - SNR_limit_on: bool = None + snr_limit_on: bool = None """flag for when an SNR limit is given""" mag_limit: float = None @@ -724,7 +724,7 @@ class expertConfigs: trailing_losses_on: bool = None """flag for trailing losses""" - default_SNR_cut: bool = None + default_snr_cut: bool = None """flag for default SNR""" randomization_on: bool = None @@ -749,14 +749,14 @@ def _validate_expert_configs(self): ---------- None """ - if self.SNR_limit is not None: - self.SNR_limit = cast_as_float(self.SNR_limit, "SNR_limit") - self.SNR_limit_on = True - if self.SNR_limit < 0: + if self.snr_limit is not None: + self.snr_limit = cast_as_float(self.snr_limit, "SNR_limit") + self.snr_limit_on = True + if self.snr_limit < 0: logging.error("ERROR: SNR limit is negative.") sys.exit("ERROR: SNR limit is negative.") else: - self.SNR_limit_on = False + self.snr_limit_on = False if self.mag_limit is not None: self.mag_limit = cast_as_float(self.mag_limit, "mag_limit") @@ -767,7 +767,7 @@ def _validate_expert_configs(self): else: self.mag_limit_on = False - if self.mag_limit_on and self.SNR_limit_on: + if self.mag_limit_on and self.snr_limit_on: logging.error( "ERROR: SNR limit and magnitude limit are mutually exclusive. Please delete one or both from config file." ) @@ -778,7 +778,7 @@ def _validate_expert_configs(self): self.trailing_losses_on = cast_as_bool_or_set_default( self.trailing_losses_on, "trailing_losses_on", True ) - self.default_SNR_cut = cast_as_bool_or_set_default(self.default_SNR_cut, "default_SNR_cut", True) + self.default_snr_cut = cast_as_bool_or_set_default(self.default_snr_cut, "default_snr_cut", True) self.randomization_on = cast_as_bool_or_set_default(self.randomization_on, "randomization_on", True) self.vignetting_on = cast_as_bool_or_set_default(self.vignetting_on, "vignetting_on", True) @@ -1409,12 +1409,12 @@ def PrintConfigsToLog(sconfigs, cmd_args): else: pplogger.info("Saturation limit is turned OFF.") - if sconfigs.expert.SNR_limit_on: - pplogger.info("The lower SNR limit is: " + str(sconfigs.expert.SNR_limit)) + if sconfigs.expert.snr_limit_on: + pplogger.info("The lower SNR limit is: " + str(sconfigs.expert.snr_limit)) else: pplogger.info("SNR limit is turned OFF.") - if sconfigs.expert.default_SNR_cut: + if sconfigs.expert.default_snr_cut: pplogger.info("Default SNR cut is ON. All observations with SNR < 2.0 will be removed.") if sconfigs.expert.mag_limit_on: From 2ca633840ca916f1fe6873d664961cf4923f9db4 Mon Sep 17 00:00:00 2001 From: Ryan Lyttle Date: Thu, 9 Jan 2025 17:34:31 +0000 Subject: [PATCH 7/9] updated unit tests changed SNR to snr in unit tests and fixed a typo --- src/sorcha/utilities/sorchaConfigs.py | 2 +- tests/sorcha/test_PPAddUncertainty.py | 2 +- tests/sorcha/test_PPRandomizeMeasurements.py | 2 +- tests/sorcha/test_sorchaConfigs.py | 18 +++++++++--------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/sorcha/utilities/sorchaConfigs.py b/src/sorcha/utilities/sorchaConfigs.py index 8d37dc99..db9c1e74 100644 --- a/src/sorcha/utilities/sorchaConfigs.py +++ b/src/sorcha/utilities/sorchaConfigs.py @@ -750,7 +750,7 @@ def _validate_expert_configs(self): None """ if self.snr_limit is not None: - self.snr_limit = cast_as_float(self.snr_limit, "SNR_limit") + self.snr_limit = cast_as_float(self.snr_limit, "snr_limit") self.snr_limit_on = True if self.snr_limit < 0: logging.error("ERROR: SNR limit is negative.") diff --git a/tests/sorcha/test_PPAddUncertainty.py b/tests/sorcha/test_PPAddUncertainty.py index 97eefc80..4e9614dd 100644 --- a/tests/sorcha/test_PPAddUncertainty.py +++ b/tests/sorcha/test_PPAddUncertainty.py @@ -104,7 +104,7 @@ def test_addUncertainties(): } ) - configs = {"trailing_losses_on": True, "default_SNR_cut": False} + configs = {"trailing_losses_on": True, "default_snr_cut": False} configs = expertConfigs(**configs) setattr(configs, "expert",configs) diff --git a/tests/sorcha/test_PPRandomizeMeasurements.py b/tests/sorcha/test_PPRandomizeMeasurements.py index 777e6cc2..befc1ff9 100644 --- a/tests/sorcha/test_PPRandomizeMeasurements.py +++ b/tests/sorcha/test_PPRandomizeMeasurements.py @@ -111,7 +111,7 @@ def test_randomizeAstrometryAndPhotometry(): observations = pd.DataFrame(data_in, index=[0]) - configs = {"default_SNR_cut": True, "trailing_losses_on": True} + configs = {"default_snr_cut": True, "trailing_losses_on": True} configs = expertConfigs(**configs) setattr(configs,"expert",configs) obs_out = randomizeAstrometryAndPhotometry(observations, configs, PerModuleRNG(2021)) diff --git a/tests/sorcha/test_sorchaConfigs.py b/tests/sorcha/test_sorchaConfigs.py index 8543de5d..4d6f8bb3 100644 --- a/tests/sorcha/test_sorchaConfigs.py +++ b/tests/sorcha/test_sorchaConfigs.py @@ -95,12 +95,12 @@ correct_activity = {"comet_activity": None} correct_expert = { - "SNR_limit": None, - "SNR_limit_on": False, + "snr_limit": None, + "snr_limit_on": False, "mag_limit": None, "mag_limit_on": False, "trailing_losses_on": True, - "default_SNR_cut": True, + "default_snr_cut": True, "randomization_on": True, "vignetting_on": True, } @@ -883,7 +883,7 @@ def test_outputConfigs_inlist(key_name, expected_list): @pytest.mark.parametrize("key_name", ["position_decimals", "magnitude_decimals"]) def test_outputConfigs_decimel_check(key_name): """ - Checks that if deciamels are not int or are negative it is caught correctly + Checks that if decimals are not int or are negative it is caught correctly """ correct_output = { "output_format": "csv", @@ -963,7 +963,7 @@ def test_activity_config(): # expert config test -@pytest.mark.parametrize("key_name", ["SNR_limit", "mag_limit"]) +@pytest.mark.parametrize("key_name", ["snr_limit", "mag_limit"]) def test_expert_config_float(key_name): """ tests that wrong inputs for expertConfigs float attributes is caught correctly @@ -977,7 +977,7 @@ def test_expert_config_float(key_name): assert error_text.value.code == f"ERROR: expected a float for config parameter {key_name}. Check value in config file." -@pytest.mark.parametrize("key_name, error_name", [("SNR_limit", "SNR"), ("mag_limit", "magnitude")]) +@pytest.mark.parametrize("key_name, error_name", [("snr_limit", "SNR"), ("mag_limit", "magnitude")]) def test_expert_config_bounds(key_name, error_name): """ Tests that values in expertConfigs are creating error messages when out of bounds @@ -993,12 +993,12 @@ def test_expert_config_bounds(key_name, error_name): def test_expert_config_exclusive(): """ - Makes sure that when both SNR limit and magnitude limit are specified that an error occurs + Makes sure that when both snr limit and magnitude limit are specified that an error occurs """ expect_configs = correct_expert.copy() expect_configs["mag_limit"] = 5 - expect_configs["SNR_limit"] = 5 + expect_configs["snr_limit"] = 5 with pytest.raises(SystemExit) as error_text: test_configs = expertConfigs(**expect_configs) @@ -1009,7 +1009,7 @@ def test_expert_config_exclusive(): @pytest.mark.parametrize( - "key_name", ["trailing_losses_on", "default_SNR_cut", "randomization_on", "vignetting_on"] + "key_name", ["trailing_losses_on", "default_snr_cut", "randomization_on", "vignetting_on"] ) def test_expertConfig_bool(key_name): """ From 48f4feeef29aa350a860ceb87f8847080658d338 Mon Sep 17 00:00:00 2001 From: Ryan Lyttle Date: Fri, 10 Jan 2025 12:08:24 +0000 Subject: [PATCH 8/9] Update demo_MagnitudeAndSNRCuts.ipynb --- docs/notebooks/demo_MagnitudeAndSNRCuts.ipynb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/notebooks/demo_MagnitudeAndSNRCuts.ipynb b/docs/notebooks/demo_MagnitudeAndSNRCuts.ipynb index 7dd0e988..70abbf65 100644 --- a/docs/notebooks/demo_MagnitudeAndSNRCuts.ipynb +++ b/docs/notebooks/demo_MagnitudeAndSNRCuts.ipynb @@ -91,7 +91,7 @@ "metadata": {}, "outputs": [], "source": [ - "test_data_maglimit = PPMagnitudeLimit(test_data, 21.)" + "test_data_maglimit = PPMagnitudeLimit(test_data, 21.,colname='PSFMag')" ] }, { From 8e5072f2a9ed00b31b086bb7fa07d55f7427ea26 Mon Sep 17 00:00:00 2001 From: Ryan Lyttle Date: Fri, 10 Jan 2025 12:24:01 +0000 Subject: [PATCH 9/9] Updated notebook Changed default_SNR_cut to be lowercase in demo notebook --- docs/notebooks/demo_UncertaintiesAndRandomization.ipynb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/notebooks/demo_UncertaintiesAndRandomization.ipynb b/docs/notebooks/demo_UncertaintiesAndRandomization.ipynb index caffab75..9a8ff76c 100644 --- a/docs/notebooks/demo_UncertaintiesAndRandomization.ipynb +++ b/docs/notebooks/demo_UncertaintiesAndRandomization.ipynb @@ -99,7 +99,7 @@ "metadata": {}, "outputs": [], "source": [ - "configs = {'trailing_losses_on':True, 'default_SNR_cut': False}\n", + "configs = {'trailing_losses_on':True, 'default_snr_cut': False}\n", "configs = expertConfigs(**configs)\n", "setattr(configs, \"expert\", configs)\n", "rng = PerModuleRNG(2012)"