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

Update of the LST1 MC telescope model #4

Open
2 tasks done
moralejo opened this issue Dec 9, 2021 · 22 comments
Open
2 tasks done

Update of the LST1 MC telescope model #4

moralejo opened this issue Dec 9, 2021 · 22 comments

Comments

@moralejo
Copy link
Collaborator

moralejo commented Dec 9, 2021

Task list of the needed updates for the LST1 model in sim_telarray

@mexanick
Copy link
Collaborator

mexanick commented Dec 9, 2021

I've pushed the default prod5 configuration to sim_telarray branch and created a corresponding tag with it
Now all the modifications can be done on that branch (or, if multiple, a separate branch e.g. sim_telarray_prod5.1 or so can be created)

@moralejo
Copy link
Collaborator Author

moralejo commented Dec 9, 2021

Thanks @mexanick. A couple of comments:

  • I think we should limit this to MAGIC1, MAGIC2 and LST1, so we can remove the LST2, 3 & 4 files, right?
  • How shall we proceed with the file names? We have now files with "PROD4" in their names, others with PROD5". But this will be none of them. Most likely the settings we decide here will end up in CTA's full-arrays PROD6. So this becomes a bit confusing... Any suggestion?

@jsitarek
Copy link
Collaborator

Hi @moralejo, about the naming, calling it PROD6 before the actual PROD6 can be very misleading, in particular if any LST settings get changed in the meantime, so I would stick to PROD5 in the naming, maybe with some extra letter? Even if the same settings end up in PROD6 and thus we have two different names for it, I think it is not a huge problem.
Perfectly fine to get rid of LST2-4, if we need any 4LST studies, we can always them with previous MCs

@moralejo
Copy link
Collaborator Author

I agree @jsitarek, just mentioned PROD6 because it adds to the confusion... I have no good suggestion as of now. Probabñy PROD5 with some additional tag would work. Anyway the versioning from Git will guarantee reproducibility.

@mexanick
Copy link
Collaborator

I'd add a minor version number, like PROD5.1 And perhaps we will have few of them (e.g. with different PSF or trigger or telescope transmission values) and can subsequently name them 5.2, 5.3 etc. And when the CTA bumps the major version to 6, we can do as well

@maxnoe
Copy link
Member

maxnoe commented Dec 10, 2021

Should we just start with our own, independent numbering? Let's call this something like LSTPROD1.

@mexanick
Copy link
Collaborator

I don't have a strong opinion, but I have two concerns: first, it may confuse people who used previous CTA productions, second, our LST prototype settings will most probably go into the CTA PROD6 and it is a bit weird to have e.g. LSTPROD1 and CTA PROD6 having the same configuration for LST-1...

@maxnoe
Copy link
Member

maxnoe commented Dec 10, 2021

I would find it more confusing to have the same prod number for different settings.

it is a bit weird to have e.g. LSTPROD1 and CTA PROD6 having the same configuration for LST-1...

Same thing already happened, PROD4 LST identical to PROD5

@mexanick
Copy link
Collaborator

I would find it more confusing to have the same prod number for different settings.

Well, that's why I propose to have minor version numbers. If the configurations differ but a small subset of parameters, then it is a minor bump, if difference is large - a major. Though, it is not very straightforward to differentiate what's small and what's large difference ...

Same thing already happened, PROD4 LST identical to PROD5

yup... not too good...

But whatever, let's wait for opinions of others :)

@moralejo
Copy link
Collaborator Author

In order to clarify a bit the situation, @mexanick can you please delete from the sim_tel branch the files that are not needed? (LST2-4 and whatever else, if anything)

@mexanick
Copy link
Collaborator

I should leave the placeholders/array layout for LST1 and two MAGICs, right?
I can do this, just need the confirmation.

@rlopezcoto
Copy link
Collaborator

Let's use PROD5b

@moralejo
Copy link
Collaborator Author

Let's use PROD5b

Prod5b has been used in CTA productions.
We could use LSTPROD2 (or 1, leaving 0 for the earlier limited production)

@moralejo
Copy link
Collaborator Author

Hi all, we have to converge on this. Naming is important, but as long as everything that is used is in the end released in this repo, it will not be critical.

Shall we go for LST-PROD2? This seems to be in use already in the lst-sim-config branch. I see no problem in keeping files which will not change at all with their original naming like PROD4, PROD5 or whatever.

As for LST config, I think there is nothing yet to be decided, we converged on the mirror dish settings: #5

But this is not yet in the lst-sim-config branch. Can someone put it there? May be @satoshifukami0115? We would need a LST-PROD2-LST.cfg file if I am not mistaken, and link it wherever needed in the other files (removing the "PROD4" equivalent we currently have there).

@satoshifukami0115
Copy link
Collaborator

But this is not yet in the lst-sim-config branch. Can someone put it there? May be @satoshifukami0115? We would need a LST-PROD2-LST.cfg file if I am not mistaken, and link it wherever needed in the other files (removing the "PROD4" equivalent we currently have there).

Hi, I might have not understood but what should be done is to copy CTA-PROD4-LST.cfg to LST-PROD2-LST.cfg with the updated optics values and the other files originally linking to CTA-PROD4-LST.cfg, right? Then I can do it soon. And just to confirm, can I push it directly to sim-telarray branch, or create a new branch to open a new PR to sim-telarray branch?

@maxnoe
Copy link
Member

maxnoe commented Jan 28, 2022

I see no problem in keeping files which will not change at all with their original naming like PROD4, PROD5 or whatever.

At least for me personally, it was always very hard to find the right config file in the forest of simtel array configurations and quite suprising when something like PROD5b-LA-PALMA included LST-PROD4-<something>.

If it's not too much to ask, I'd very much like a consistent naming scheme.

However since we are working now with a git based scheme based on tags, maybe we should not include any version specification in the config files and let git take care of the versioning.

@moralejo
Copy link
Collaborator Author

moralejo commented Jan 28, 2022

However since we are working now with a git based scheme based on tags, maybe we should not include any version specification in the config files and let git take care of the versioning.

Sounds very reasonable to remove any versioning from the file names.

For the record, should we include, in the comments inside the file, the name of the "parent" of the file from the earlier production? (the contents of the file may be identical to the parent's, or contain some slight modifications). And after the first release, we just need the github tags (and can remove any comments about the original file names).

@moralejo
Copy link
Collaborator Author

But this is not yet in the lst-sim-config branch. Can someone put it there? May be @satoshifukami0115? We would need a LST-PROD2-LST.cfg file if I am not mistaken, and link it wherever needed in the other files (removing the "PROD4" equivalent we currently have there).

Hi, I might have not understood but what should be done is to copy CTA-PROD4-LST.cfg to LST-PROD2-LST.cfg with the updated optics values and the other files originally linking to CTA-PROD4-LST.cfg, right? Then I can do it soon. And just to confirm, can I push it directly to sim-telarray branch, or create a new branch to open a new PR to sim-telarray branch?

Hi @satoshifukami0115 , you have write access so you can push to the branch. But let's see if there are other opinions on file names. Perhaps we should call that just LST1.cfg

@satoshifukami0115
Copy link
Collaborator

satoshifukami0115 commented Jan 28, 2022

Hi, I just uploaded tentatively the LST1.cfg with the new optics value (with random mirror align 0.0046). I will again confirm the other optics parameters with Jakub.

@moralejo
Copy link
Collaborator Author

Thanks @satoshifukami0115
I don't really know the "connections" among all the configuration files (which ones link to others), or whether all the files in the branch are really needed or not. We need someone who can clean that up, and rename the files (if we agree to remove "production tags" from the file names). Probably @jsitarek is the right person to do that for MAGIC, and @Voutsi and @mexanick for LST?

@mexanick
Copy link
Collaborator

The connections are done through master configuration file:
https://github.com/cta-observatory/lst-sim-config/blob/sim-telarray/CTA-PROD5-LaPalma-baseline_4LSTs_MAGIC.cfg
I'm currently at La Palma for shifts, but will try to find some time to clean up the repo in coming days

@jsitarek
Copy link
Collaborator

Hi @moralejo, sure I will do the cleaning and renaming of the MAGIC part. But I would like to discuss first what to do with the reflectivity file, since I also saw some small differences there and best to discuss it during the Wednesday MAGIC+LST1 meeting, so is it fine if you do it afterwards?

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

No branches or pull requests

6 participants