-
Notifications
You must be signed in to change notification settings - Fork 1
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
start SOM plugin #313
base: master
Are you sure you want to change the base?
start SOM plugin #313
Conversation
amstrax/contexts.py
Outdated
"""XENONnT context for the SOM.""" | ||
|
||
st = ax.contexts.xams(**kwargs) | ||
del st._plugin_class_registry["peaks"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very dangerous. What are you trying to do? I would redefine the context creation code for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same way we set-up this context in straxen, why do you say its dangerous?
The PR is already in github, but the problem is the following. Similarly to how it was done in straxen I made a new plugin -> PeaksSOM and a context under which this plugin can function, I had to change some of the details due to the different structures between the two but I am running into a mailbox issue coming from strax which claims the argument 'records' were given to compute. However, the PeaksSOM is trying to first compute what was previously under "Peaks". And the peaks plugin does have a compute argument with records however it doesnt make sense that the current SOM plugin flags this error since there was a computation of 'records' before I made any changes. I will include the output error as well as an example of how I triggered it:
|
@LuisSanchez25 We cannot run this without the xams_som_v1.npz… please add. |
This should help:
|
Ah I assumed people here would have access to the stoomboot machine which is where the file is saved, I can add it in the PR directly |
Pull Request Test Coverage Report for Build 9118375458Details
💛 - Coveralls |
Hey @JelleAalbers thanks a lot for your help! I was misunderstanding how the super function works, I think I can fix the rest from here thanks a lot! |
This plugin will be the start for me to modify amstrax in a way that we can use to process data with the SOM classification