-
Notifications
You must be signed in to change notification settings - Fork 8
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
add fraction threshold to global settings file #27
Comments
I'll add the possibility to define a fraction threshold parameter to the global settings file once the new energy offset code has been merged |
copy pasted from the merge: Hi you two, But I agree, that it would be nice to have a global settings file, maybe per RE-EDS project, containing the desired value for this. |
also copied from the merge ;) yes, that would also be my suggestion, that we add it to global_definitions.py, and then it can be changed from there |
then I will close #10 because this is a duplicate :) right? |
I was just thinking of adding it as a variable to the already existing global_definitions.py. But that's not what you had in mind with #10 , right? |
Ah you are right! But I think your idea is very good and efficient. ;) |
The global definitions file was an idea I had some time ago, but never had the time to realise it. |
This was added by @schroederb in #32 . Thanks a lot for taking care of it! |
see #18:
Personally, I would prefer to have a fixed default value, and adapt it from the input scripts - I think it makes it easier to have a consistent main branch and not accidentally merge other default values from out own branches without other users noticing.
If you don't like adapting it in multiple places, which is understandable, we could add an option to add it to the global definition file, so you only have to change it in one place and then the other submission scripts take the value from there.
But if you both think it's nicer to change the threshold directly in the function, that's of course also fine for me :)
Originally posted by @SalomeRonja in #18 (comment)
The text was updated successfully, but these errors were encountered: