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

hotspot veto #93

Merged
merged 4 commits into from
Jul 18, 2023
Merged

hotspot veto #93

merged 4 commits into from
Jul 18, 2023

Conversation

LuisSanchez25
Copy link
Contributor

I think this might be the first a few time dependent cuts people will want to add to xedocs, should we make a separate folder for these cuts since they are not really corrections?

@LuisSanchez25 LuisSanchez25 marked this pull request as ready for review July 12, 2023 19:13
Copy link
Member

@jmosbacher jmosbacher left a comment

Choose a reason for hiding this comment

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

Wondering if "corrections" is the correct category for this schema. The corrections are immutable, are we sure this is needed?
Also, take a look at the hotspot report schema under "operations_reports", are you sure this cant be used for cuts?

@LuisSanchez25
Copy link
Contributor Author

From my understanding with what Kexin was trying to do it seems like she wanted to have an individual value to be used for a certain number of runs (those with hotspots). I can ask for more details/clarification to see if she does want these values to be immutable and make new version in the future or not.
Looking at the hotspot file under "operations_reports", I dont think it covers what she had in mind but Ill talk to her.

@jmosbacher
Copy link
Member

@LuisSanchez25 I suggest that you invite the analysts that requested this schema to participate in the discussion here so we have a proper record of the development.

@LuisSanchez25
Copy link
Contributor Author

@chnlkx

@chnlkx
Copy link
Contributor

chnlkx commented Jul 14, 2023

Hi Luis and Yossi! Thanks for starting this PR : )

  • As Luis mentioned, what we trying to put into the time-dependent corrections is a series of single values of the hotspot_veto_cut threshold for each run throughout the science run. These values would be immutable and versioned.
  • And for the hotspot file under "operations_reports", from the content I think it's for the operation log purpose, maintained by the shifters, and only involves a small number of runs. Yet the one we are trying to set up is an outcome of the offline analysis and would involve all the science data runs. So from my understanding, they are not suitable to combine due to different usage scenarios.

@jmosbacher
Copy link
Member

@chnlkx thanks for the extra details. So if i understand you correctly, this collection will store a threshold number that will be used to cut data that is above this threshold? what config in the cut plugin will this value be used for?
If this is the case, I suggest renaming the schema to something more suitable to its use and adding some more info in the module/class docstring

@chnlkx
Copy link
Contributor

chnlkx commented Jul 18, 2023

@jmosbacher Thanks for your comment!

So if i understand you correctly, this collection will store a threshold number that will be used to cut data that is above this threshold?

Correct!

What config in the cut plugin will this value be used for?

I'm trying to use the the URL config in the cut plugin like:

cut_threshold = straxen.URLConfig( default="resource://xedocs://hotspot_veto_threshold?attr=value&run_id=plugin.run_id&version=v1" help='Upper boundary of SE density [Hz/cm^2]')
Is it proper?
And I also modified the name and docstring of the schema.

@jmosbacher
Copy link
Member

@chnlkx in your URL example, you are passing the value of the xedoc document to the resource:// protocol which is usually used to fetch a file stored in the private_nt_aux repo. If the value of cut_threshold should be just a number then you can use "xedocs://hotspot_veto_threshold?attr=value&run_id=plugin.run_id&version=v1" as the default URL.

jmosbacher
jmosbacher previously approved these changes Jul 18, 2023
Copy link
Member

@jmosbacher jmosbacher left a comment

Choose a reason for hiding this comment

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

I think this looks fine.

xedocs/schemas/corrections/hotspot_veto_cut.py Outdated Show resolved Hide resolved
@jmosbacher jmosbacher merged commit cf61b1d into master Jul 18, 2023
4 checks passed
@chnlkx chnlkx deleted the hospot_veto_cut branch July 18, 2023 21:42
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.

3 participants