Skip to content

Conversation

@timomit
Copy link
Collaborator

@timomit timomit commented Apr 4, 2025

the dataloader takes a list of pattern. These pattern are internally randomly shuffled and loaded in a lazy way.

@timomit timomit added the enhancement New feature or request label Apr 4, 2025
@timomit timomit requested a review from Benano April 4, 2025 13:29
@timomit timomit self-assigned this Apr 4, 2025
timomit and others added 6 commits April 4, 2025 17:16
changed field.name to f.name
for multiple patterns. Feels a bit like an overkill. :P
and teh respective tests for the ShuffleDataloader
@timomit timomit force-pushed the FeatureMultiPattern branch from c5c464a to 40e9b5f Compare April 8, 2025 13:01
Copy link
Collaborator

@Benano Benano left a comment

Choose a reason for hiding this comment

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

Very nicely written code of an elegant solution!

# self.pattern[0].transform(pre_transforms[0])
# print(self.pattern[0])

self._rng = np.random.default_rng(seed)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I remember J telling me that its good practice to define the randomness generator outside of the class and to pass the generator rather than passing a seed.


# now, fill the arrays until t_max
# we do this, because the pattern can have arbitrary lengths:
while self._starting_times[-1] < t_max:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this approach a lot. Having all of the starting times is super handy!

current_pat_idx = self.choice()
self._pat_ids.append(current_pat_idx)
self._starting_times.append(
self._starting_times[-1] + self.durations[self._pat_ids[-2]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't quite understand what is happening here. Why are you adding only the duration of the second to last pattern? Lets quickly discuss.

Apply online transformations to a pattern.
This method applies all specified online transformations in sequence
to the given 1-dimensional pattern.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here I was confused for a moment why it has to be a 1D pattern but I understand now thats it is a 1D slice of the pattern. Consider renaming :)

raise ValueError("t must be small smaller than t_max.")
# find index of the pattern that is at t:
t += offset
idx_in_sequence = np.searchsorted(self._starting_times, t) - 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

What a handy function! I did not know about it.

Copy link
Collaborator

@Benano Benano left a comment

Choose a reason for hiding this comment

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

Fix rng handling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants