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

Clarification on prediction_mask in MoiraiFinetune #29

Closed
zqiao11 opened this issue Apr 19, 2024 · 5 comments · Fixed by #49
Closed

Clarification on prediction_mask in MoiraiFinetune #29

zqiao11 opened this issue Apr 19, 2024 · 5 comments · Fixed by #49
Assignees
Labels
bug Something isn't working

Comments

@zqiao11
Copy link

zqiao11 commented Apr 19, 2024

Hi @gorold. May I clarify the setup of the prediction mask in validation dataset in MoiraiFinetune?

            + EvalMaskedPrediction(
                mask_length=-prediction_length % patch_size,  # num of patches for prediction?
                target_field="target",
                truncate_fields=("variate_id", "time_id", "observed_mask"),
                optional_truncate_fields=("past_feat_dynamic_real",),
                prediction_mask_field="prediction_mask",
                expected_ndim=3,
            )

Now mask_length is computed as -prediction_length % patch_size. Shouldn't the mask_length be computed as prediction_length // patch_size? As the logic here seems to mask the patches in the prediction range.

    def _generate_prediction_mask(
        self, target: Float[np.ndarray, "var time *feat"]
    ) -> Bool[np.ndarray, "var time"]:
        self.check_ndim("target", target, self.expected_ndim)
        var, time = target.shape[:2]
        prediction_mask = np.zeros((var, time), dtype=bool)
        prediction_mask[:, -self.mask_length :] = True
        return prediction_mask

Please correct me if my understanding is incorrect. Look forward to your reply.

@gorold
Copy link
Contributor

gorold commented Apr 19, 2024

Hey, thanks for catching this. I think you're right, it should be prediction_length // patch_size, will look into this.

@gorold gorold self-assigned this Apr 19, 2024
@gorold gorold added the bug Something isn't working label Apr 19, 2024
@zqiao11
Copy link
Author

zqiao11 commented May 7, 2024

Hi, it should be math.ceil(prediction_length / patch_size) since the prediction range has been padded.

@gorold
Copy link
Contributor

gorold commented May 7, 2024

I think there wouldn't be a difference if the prediction range is already padded?

@zqiao11
Copy link
Author

zqiao11 commented May 7, 2024

Let's say if prediction length is 96 and patch size is 64, the prediction range will be padded to 128 to make sure there are multiple patches. So, the last 2 patches should be masked as True in prediction_mask.

If we use prediction_length // patch_size, then only the last patch is masked as True.

@gorold
Copy link
Contributor

gorold commented May 7, 2024

Right... this makes sense. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants