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

Add YBe Generator #134

Merged
merged 9 commits into from
May 3, 2024
Merged

Add YBe Generator #134

merged 9 commits into from
May 3, 2024

Conversation

ghusheng
Copy link
Collaborator

@ghusheng ghusheng commented May 3, 2024

Add generator_ybe in the generator.py , the default YBe instruction file path is /project2/lgrandi/ghusheng/ybe_instrutions/ybe_wfsim_instructions_6806_events_time_modified.csv

@ghusheng
Copy link
Collaborator Author

ghusheng commented May 3, 2024

Maybe to combine ambe and ybe generator (also maybe neutron_gun in the future) to nr_generator?

@FaroutYLq FaroutYLq self-requested a review May 3, 2024 16:08
Copy link
Collaborator

@FaroutYLq FaroutYLq left a comment

Choose a reason for hiding this comment

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

Thanks a lot @ghusheng. I would just keep the generator itself and remove other things. One thing we might need to think about is to put all the csv inputs (together with the AmBe's) into a more shared-like folder something like: /project/lgrandi/xenonnt/saltax_inputs. Maybe in another PR.
Meanwhile, since AmBe and YBe are so different in energy and positions I would avoid putting them into one generator. It makes no sense to me to sprinkle ambe neutrons into a ybe runs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This should go

jobs/config.ini Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would keep the template so not to touch this

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would keep the original config as a template

Copy link
Collaborator

Choose a reason for hiding this comment

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

This should go

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks good to me, but I am a bit lazy to test, so I would just trust you:)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let me do a minor change

@FaroutYLq
Copy link
Collaborator

Ignore the tests, they are nonsense for now.

@ghusheng
Copy link
Collaborator Author

ghusheng commented May 3, 2024

should be all good now

@FaroutYLq FaroutYLq merged commit ceafb9f into main May 3, 2024
1 of 2 checks passed
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.

2 participants