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

Refactoring in support of externally defined feature extractor libraries - i.e. LAISS #73

Merged
merged 5 commits into from
Nov 8, 2024

Conversation

drewoldag
Copy link
Collaborator

This fairly substantial refactor removes hardcoded column names from lightcurve_utils and shifts the work over to the subclasses of LightCurve. The built in feature extractors (FEs) all use the same patterns, so we were able to keep the logic in the parent class.

This reduces the code in RESSPECT, and leaves the door open for external libraries that bring in a FE to implement their own method of providing feature column names.

…ghtcurve_utils`. Dynamically generate the column names as needed using methods defined in the `LightCurve` feature extractor subclasses.
@drewoldag drewoldag self-assigned this Nov 1, 2024
Copy link

github-actions bot commented Nov 1, 2024

Before [7c56dc6] After [9650d24] Ratio Benchmark (Parameter)
151±0.5ms 153±0.4ms 1.01 benchmarks.time_learn_loop('KNN', 'RandomSampling')
132±0.6ms 132±2ms 1 benchmarks.time_feature_creation
2.60±0.01s 2.59±0.02s 1 benchmarks.time_learn_loop('RandomForest', 'RandomSampling')
197M 195M 0.99 benchmarks.peakmem_learn_loop('KNN')
188M 186M 0.99 benchmarks.peakmem_learn_loop('RandomForest')
154±0.6ms 153±0.9ms 0.99 benchmarks.time_learn_loop('KNN', 'UncSampling')
2.64±0.02s 2.62±0.01s 0.99 benchmarks.time_learn_loop('RandomForest', 'UncSampling')

Click here to view all benchmarks.

src/resspect/database.py Outdated Show resolved Hide resolved
@@ -234,6 +239,10 @@ def load_features_from_file(self, path_to_features_file: str, screen=False,
"Bump", or "Malanchev". Default is "Bazin".
"""

if survey not in ['DES', 'LSST']:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@AmandaWasserman Is this logic correct, or should we support any survey that we find in the FILTER_SETS dictionary? i.e. DES, LSST, and SNPCC

drewoldag and others added 3 commits November 6, 2024 16:26
- Image installs resspect from source
- Image builds on latest ubuntu avoiding conflicts with ubuntu's version of pip
- Image sets up a data directory and a venv for resspect
- Image does not yet work with the docker-compose file or the TOM docker-compose ecosystem.
And document how to connect to tom docker-compose ecosystem
@AmandaWasserman
Copy link
Collaborator

Everything looks good doing a PR and run through of our typical operations for feature extraction, learn loop, and plotting

@drewoldag drewoldag merged commit a1b8a6f into main Nov 8, 2024
8 checks passed
@drewoldag drewoldag deleted the issue/72/refactor-feature-extractor-column-names branch November 8, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Additional refactoring to handle column names for feature extractors in external libraries
4 participants