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

SpikeSortingRecordingSelection attributes #133

Open
magland opened this issue Feb 25, 2022 · 5 comments · Fixed by #134
Open

SpikeSortingRecordingSelection attributes #133

magland opened this issue Feb 25, 2022 · 5 comments · Fixed by #134
Assignees

Comments

@magland
Copy link
Contributor

magland commented Feb 25, 2022

I was noticing that LabTeam is not a primary key for SpikeSortingRecordingSelection (see below).

Does that mean that if one lab team does spike sorting with particular sort group, sort interval, and sorting parameters, then another lab team cannot also do that sorting? Seems not right... unless the lab team is associated with the session, in which case it should not be an attribute of this table.

@schema
class SpikeSortingRecordingSelection(dj.Manual):
    definition = """
    # Defines recordings to be sorted
    -> SortGroup
    -> SortInterval
    -> SpikeSortingPreprocessingParameters
    ---
    -> IntervalList
    -> LabTeam
    """
@khl02007
Copy link
Collaborator

@magland thanks, that's a good point. I think we initially had implemented the lab team system because some people in the lab did not want everyone else to have access to their nwb file, but we certainly do want to encourage sharing. @lfrank what do you think? I can move LabTeam to the primary key if you're OK with it.

@lfrank
Copy link
Contributor

lfrank commented Feb 25, 2022 via email

@CBroz1
Copy link
Member

CBroz1 commented Nov 8, 2024

@khl02007 It's seems like it's currently the case that a second team running an otherwise identical key will cause the first run to be deleted:

recording_path = str(recording_dir / Path(recording_name))
if os.path.exists(recording_path):
shutil.rmtree(recording_path)

Do we want to fix that by adding team to the folder name? I'm thinking there should be a prompt before deleting another team's folder

Ignoring team, there are currently no duplicate keys, so no cause for concern about overwritten files. But does the team influence the outcome? Or is the process deterministic, such that the same parameters will yield the same result? I'm revisiting this table with #917 in mind

It team just indicates run ownership, I think we now navigate that better with cautious delete style ownership. I wouldn't propose edits to this table, but just trying to wrap my head around the need discussed here to both tackle recompute and consider for future pipeline development

@khl02007
Copy link
Collaborator

@CBroz1 I see, I didn't realize this issue, thanks for catching it. SpikeSortingRecording.make is supposed to be deterministic, and would give you the same results regardless of which team runs it. I think there is little point in running SpikeSortingRecording.make on the same set of parameters when someone else has done it. I think the labteam name was put in as a secondary key to protect against a nonmember of the team deleting the entry. Does this help you make decisions about how to change it?

@CBroz1
Copy link
Member

CBroz1 commented Nov 13, 2024

That's helpful, thanks. I'll reopen pending a fix of the possibility of the folder overwrite

I think this rmtree should be replaced with an insert that points to the existing folder

@CBroz1 CBroz1 reopened this Nov 13, 2024
@CBroz1 CBroz1 self-assigned this Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants