-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implement new abstract lephare functions #27
Implement new abstract lephare functions #27
Conversation
We would liek the option to run autoadapt either on the inform training set or on each chunk
We might want to run the get data which would also close #12 Further we should add the autoadapt stage to the inform stage. In general we will not have access to spectroscopic redshifts in the estimate stage. |
Still to implement autoadapt in inform stage. We need to convert the config key map to a dictionary for it to be pickled. It can easily be converted back by looping over the keys and replacing the values with lephare.keyword objects
I left get data out of the inform stage so it must be run outside of rail. |
This depends on the changes to DataManager in lephare being merged and published. There may be situations where the user wants to default to imed runs but currently they have to do this via the name of the inform stage
I think the tests are failing because the lephare version doesn't yet have the change to create_new_run which we now use to set the run equal to the informer name. |
Yep, that makes sense. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I think this looks just fine. I'm sure there are corner cases and issues that we haven't uncovered yet, but I would advocate for moving to get it in to main so that we can all experiment with it together.
src/rail/estimation/algos/lephare.py
Outdated
self._create_filter_library() | ||
self._create_sed_library() | ||
self._create_mag_library() | ||
lp.prepare(self.lephare_config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lp.prepare(self.lephare_config) | |
lp.prepare(self.lephare_config, | |
star_config=self.config["star_mag_dict"], | |
gal_config=self.config["gal_mag_dict"], | |
qso_config=self.config["qso_mag_dict"]) |
dict( | ||
lib_ascii="YES", | ||
lp.read_config( | ||
"{}/{}".format(os.path.dirname(os.path.abspath(__file__)), "lsst.para") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this is fine - but I don't think I've seen the default value being defined as a line of code like this. i.e. lp.read_config(...). The way this was originally seemed more intuitive. With self.lephare_config = lp.read_config(self.config["lephare_config_file"])
specified in the __init__
method.
os.environ["LEPHAREDIR"] = lepharedir | ||
if lepharework is not None: | ||
os.environ["LEPHAREWORK"] = lepharework | ||
importlib.reload(lp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is working, then that's fine, I don't think it's worth poking a bear. But I really do wish we had a way around this that didn't require it.
we are trying to move most of the lephare code to lephare itself.
Closes #24
Closes #30
This will also involve changes the Params to take lephare configs to overwrite values
Do we want to run get data in the info stages?
What minimal functionality do we want to have in place prior to the in person meetings?
We would like the option to run autoadapt either on the inform training set or on each chunk
Problem & Solution Description (including issue #)
rail_lephare is running but we decided it would be neater to move some of the code to lephare in abstract functions rather than have details exposed to rail_lephare. this involves two new functions prepare and process in lephare which should be the totality of what would be run in rail_lephare.
Code Quality
#pragma: no cover
; in the case of a bugfix, a new test that breaks as a result of the bug has been added