-
Notifications
You must be signed in to change notification settings - Fork 22
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
Log Bins for snowglobes.py #206
base: main
Are you sure you want to change the base?
Conversation
Thank you for this contribution! |
python/snewpy/snowglobes.py
Outdated
#needed_offset = tedges[0] + 0.0001*u.s | ||
#tedges = tedges + tedges[0] + 0.0001*u.s #shift so it's very close to 0 | ||
#tmin = 0.0001*u.s | ||
#tmax+=tmin | ||
log_edges = np.asarray([]) | ||
|
||
if tmax < 0 or tmin < 0: |
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.
Shouldn’t these be <=0
?
I’m so sorry, but it seems I missed the forest for the trees in my earlier review: if tmax <= 0 or tmin <= 0:
raise ValueError("Cannot apply log to time windows that are less than or equal to 0. Consider adjusting model time window") The problem is that the These inconsistent and confusing arguments are definitely something we need to fix as part of #209 (which just got moved a little higher up my priority list). In the meantime, should we put this PR on hold? Or is it worth adding |
This PR is to continue the discussion in the tg3-detectorresponse slack channel regarding logarithmically spaced time bins in the
generate_time_fluence
feature insnowglobes.py
. I've also implemented @JostMigenda 'sValueError
idea for dealing with negative times. Please note that this PR is merely a prototype and it includes two files:data_handlers.py
andlog_bins.py
that are necessary to testing but are not intended to be included in the final product.