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

Producing source dependent MC production for zenith 20 deg #188

Closed
wants to merge 2 commits into from

Conversation

chaimain
Copy link
Contributor

@chaimain chaimain commented Mar 5, 2022

Starting source-dependent MC production, after which we can produce with some MC tuning in a separate PR

@codecov
Copy link

codecov bot commented Mar 5, 2022

Codecov Report

Merging #188 (fb99cdd) into master (a231233) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #188   +/-   ##
=======================================
  Coverage   15.77%   15.77%           
=======================================
  Files          35       35           
  Lines        2117     2117           
=======================================
  Hits          334      334           
  Misses       1783     1783           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a231233...fb99cdd. Read the comment docs.

@chaimain
Copy link
Contributor Author

chaimain commented Mar 5, 2022

@SeiyaNozaki Can you just confirm if the parameters are correct as we discussed?

Copy link
Contributor

@SeiyaNozaki SeiyaNozaki left a comment

Choose a reason for hiding this comment

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

Thank you for this request @chaimain !
You need to use input parameters for source-dependent analysis at the training step.
The other concerns are:

  • For source-dep analysis, point gamma MC should be used for RF training. I'm not sure where it can be configured but should be used.
  • when creating source-dep IRF, we need to use an option of --source-dep. Can this option used just by modifying the configuration file?
  • I guess dl2_to_sensitivity will not work with source-dep analysis

production_configs/srcdep_zen20/lstchain_config.json Outdated Show resolved Hide resolved
production_configs/srcdep_zen20/lstchain_config.json Outdated Show resolved Hide resolved
production_configs/srcdep_zen20/lstchain_config.json Outdated Show resolved Hide resolved
@chaimain
Copy link
Contributor Author

chaimain commented Mar 6, 2022

Thank you for this request @chaimain ! You need to use input parameters for source-dependent analysis at the training step.

Done, thanks for checking.

The other concerns are:

  • For source-dep analysis, point gamma MC should be used for RF training. I'm not sure where it can be configured but should be used.

I have removed diffuse-gamma MC from the workflow. Will that be ok, or do you want it to be only restricted in the RF training?

  • when creating source-dep IRF, we need to use an option of --source-dep. Can this option used just by modifying the configuration file?

I will raise an issue here to add such support.

  • I guess dl2_to_sensitivity will not work with source-dep analysis

For this, I have commented it from the lstmcpipe stages to be run.

@SeiyaNozaki
Copy link
Contributor

SeiyaNozaki commented Mar 7, 2022

I have removed diffuse-gamma MC from the workflow. Will that be ok, or do you want it to be only restricted in the RF training?

There is no need to use diffuse gamma, so it would be fine to remove diffuse-gamma MC, but also need to modify codes to add an option to use point-gamma MC as RF training:
https://github.com/cta-observatory/lstmcpipe/blob/master/lstmcpipe/stages/mc_train.py#L52-L53

@chaimain
Copy link
Contributor Author

We can probably close this one and the 40 deg one. Unless anyone has any objections.

@vuillaut vuillaut closed this May 20, 2022
@vuillaut vuillaut deleted the srcdep_zen20 branch April 17, 2023 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants