Skip to content

Photon peak tutorial for sector alignment #566

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

Merged
merged 14 commits into from
Apr 16, 2025
Merged

Conversation

kutnyakhov
Copy link
Collaborator

Added photon peak tutorial and new Photon_peak dataset (available on Zenodo) to fix #565

@rettigl rettigl force-pushed the ph-peak-for-sector-alignment branch from 394b273 to d75b072 Compare April 13, 2025 22:06
Copy link
Member

@rettigl rettigl left a comment

Choose a reason for hiding this comment

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

I fixed your configs etc. now, but honestly I don't understand what you are doing here. You don't actually aligne the DLD sectors, as along as I can tell, nor generate the parameters for it...
Ideally, add a routine to the processor to generate those parameters (semi) automatically.

@coveralls
Copy link
Collaborator

coveralls commented Apr 13, 2025

Pull Request Test Coverage Report for Build 14502539411

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.063%

Totals Coverage Status
Change from base Build 14445750250: 0.0%
Covered Lines: 8166
Relevant Lines: 8870

💛 - Coveralls

@rettigl
Copy link
Member

rettigl commented Apr 14, 2025

@zain-sohail @kutnyakhov The pulserSignAdc channel seems to cause problems for these files. What is it good for anyways? It is not being used anywhere in the tutorials. @steinnymir added it here: 0ef68d5

@kutnyakhov
Copy link
Collaborator Author

@zain-sohail @kutnyakhov The pulserSignAdc channel seems to cause problems for these files. What is it good for anyways? It is not being used anywhere in the tutorials. @steinnymir added it here: 0ef68d5

I agree that it is not needed for tutorials, but we have it from time to time in the datastream, e.g. last time it was used for the magnetic pulser during spin-resolved beamtime. A long time ago, that channel was used for the I0 monitor of an optical laser, but now it has its own channel opticalDiode.

@kutnyakhov
Copy link
Collaborator Author

I fixed your configs etc. now, but honestly I don't understand what you are doing here. You don't actually aligne the DLD sectors, as along as I can tell, nor generate the parameters for it... Ideally, add a routine to the processor to generate those parameters (semi) automatically.

This is basically my problem, that I was not able to understand what align_dld_sectors() is doing. Even if I was trying to feed in some crazy values, like sector_delays= [3050., 10000., 5000., 4000., 15000., 0., 0., 0.], but I could not see any changes. For me, it seems that something is missing in the code. We discussed this with @zain-sohail and I guess he had some ideas what could it be

@rettigl
Copy link
Member

rettigl commented Apr 14, 2025

I fixed your configs etc. now, but honestly I don't understand what you are doing here. You don't actually aligne the DLD sectors, as along as I can tell, nor generate the parameters for it... Ideally, add a routine to the processor to generate those parameters (semi) automatically.

This is basically my problem, that I was not able to understand what align_dld_sectors() is doing. Even if I was trying to feed in some crazy values, like sector_delays= [3050., 10000., 5000., 4000., 15000., 0., 0., 0.], but I could not see any changes. For me, it seems that something is missing in the code. We discussed this with @zain-sohail and I guess he had some ideas what could it be

It works quite correctly, however it is being applied to the dldTimeSteps, i.e. you have to call it before conversion to ns (actually not needed at all). See updated example.

kutnyakhov and others added 10 commits April 14, 2025 16:47
@rettigl rettigl force-pushed the ph-peak-for-sector-alignment branch from 5ecb3ff to 81f5c23 Compare April 14, 2025 14:48
@kutnyakhov
Copy link
Collaborator Author

I fixed your configs etc. now, but honestly I don't understand what you are doing here. You don't actually aligne the DLD sectors, as along as I can tell, nor generate the parameters for it... Ideally, add a routine to the processor to generate those parameters (semi) automatically.

This is basically my problem, that I was not able to understand what align_dld_sectors() is doing. Even if I was trying to feed in some crazy values, like sector_delays= [3050., 10000., 5000., 4000., 15000., 0., 0., 0.], but I could not see any changes. For me, it seems that something is missing in the code. We discussed this with @zain-sohail and I guess he had some ideas what could it be

It works quite correctly, however it is being applied to the dldTimeSteps, i.e. you have to call it before conversion to ns (actually not needed at all). See updated example.

Thanks for the details and updates. Now I see that it is doing something. The point with the ns conversion was to show some meaningful values, which everyone can understand and have direct comparison to, e.g. DLD time resolution.
Probably we have to add one more explanation line in the config file related to this part, that sector_delays should be configured/filled with proper numbers (in steps) before applying this function - somehow, from the explanation in tutorial 4, it sounded that this is done automatically.

@rettigl
Copy link
Member

rettigl commented Apr 15, 2025

If you don't provide values, it takes those from the config file, if they exist. Otherwise it does nothing.

@kutnyakhov
Copy link
Collaborator Author

I guess it can be merged.
P.s. @rettigl, why don't I see a tutorial for Moritz in the docs?

@rettigl
Copy link
Member

rettigl commented Apr 16, 2025

P.s. @rettigl, why don't I see a tutorial for Moritz in the docs?

See the logs:

Notebook error:
NoSuchKernel in tutorial/12_momentum_correction_of_binned_data.ipynb:
No such kernel named sed_kernel_v1

Your latest changes overwrote the kernel line, and I overlooked it. Will fix it here.

@rettigl
Copy link
Member

rettigl commented Apr 16, 2025

I agree that it is not needed for tutorials, but we have it from time to time in the datastream, e.g. last time it was used for the magnetic pulser during spin-resolved beamtime.

I disabled this now in the general config, if it is needed it can be undocumented per experiment

Copy link
Member

@rettigl rettigl left a comment

Choose a reason for hiding this comment

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

Good to go now

@rettigl rettigl merged commit 5869eca into main Apr 16, 2025
5 checks passed
@rettigl rettigl deleted the ph-peak-for-sector-alignment branch April 16, 2025 21:12
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.

Problem with align_dld_sectors
3 participants