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

Check duplicate depends_on of Plugin #859

Merged
merged 10 commits into from
Aug 1, 2024
Merged

Conversation

yuema137
Copy link
Collaborator

@yuema137 yuema137 commented Jul 26, 2024

Solve error #846.

Explanation

Basically what happened was:

  • In plugin.py, the chunks are fetched for each self.depends_on and stored in a dictionary:
    inputs = dict()
    # Fetch other inputs (when needed)
    for d in self.depends_on:
    if d != pacemaker:
    while (
    self.input_buffer[d] is None
    or self.input_buffer[d].end < this_chunk_end
    ):
    self._fetch_chunk(d, iters, check_end_not_before=this_chunk_end)
    inputs[d], self.input_buffer[d] = self.input_buffer[d].split(
    t=this_chunk_end, allow_early_split=True
    )
    # If any of the inputs were trimmed due to early splits,
    # trim the others too.
    # In very hairy cases this can take multiple passes.
    # can we optimize this, or code it more elegantly?
    max_passes_left = 10
    while max_passes_left > 0:
    this_chunk_end = min([x.end for x in inputs.values()] + [this_chunk_end])
    if len(set([x.end for x in inputs.values()])) <= 1:
    break
    for d in self.depends_on:
    inputs[d], back_to_buffer = inputs[d].split(
    t=this_chunk_end, allow_early_split=True
    )
    self.input_buffer[d] = strax.Chunk.concatenate(
    [back_to_buffer, self.input_buffer[d]],
    self.allow_hyperrun,
    )
    max_passes_left -= 1
    else:
    raise RuntimeError(
    f"{self} was unable to get time-consistent "
    f"inputs after ten passess. Inputs: \n{inputs}\n"
    f"Input buffer:\n{self.input_buffer}"
    )
  • Therefore, when there are duplicate items in self.depends_on , the later one overwrites the previous one
  • The later one cannot get anything, because the fetch is done in this way:
    inputs[d], self.input_buffer[d] = self.input_buffer[d].split(
    t=this_chunk_end, allow_early_split=True
    As the data is already read and split by the previous fetch, the later one can only get an empty result
  • So finally both ones are empty

Solution

The fix is simple, which is to remove the duplicate items in self.depends_on of the Plugin class.

@coveralls
Copy link

coveralls commented Jul 26, 2024

Coverage Status

coverage: 90.437% (+0.007%) from 90.43%
when pulling 94248d4 on fix_zero_length_repeated
into 529886d on master.

strax/plugins/plugin.py Outdated Show resolved Hide resolved
@yuema137
Copy link
Collaborator Author

yuema137 commented Aug 1, 2024

@dachengx In order to pass the pre-commit check I had to format it with black & flake. I think it happens now and then when we make PR for the files that were not checked previously. Should we do a through cleaning for strax?

strax/plugins/plugin.py Outdated Show resolved Hide resolved
strax/plugins/plugin.py Outdated Show resolved Hide resolved
strax/plugins/plugin.py Outdated Show resolved Hide resolved
@dachengx
Copy link
Collaborator

dachengx commented Aug 1, 2024

This PR was initially aimed to solve #846, but replaced by #860.

But this PR reminds us that we need to make sure that the depends_on is unique. I will change this PR to a safe guard of Plugin.

@dachengx dachengx changed the title remove duplicate depends in plugin.py Check duplicate depends_on of Plugin Aug 1, 2024
@dachengx dachengx merged commit 63100f5 into master Aug 1, 2024
9 checks passed
@dachengx dachengx deleted the fix_zero_length_repeated branch August 1, 2024 19:06
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