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

Feature/Check input for NaNs when available_mask = 1 #894

Merged
merged 8 commits into from
Feb 22, 2024
Merged

Feature/Check input for NaNs when available_mask = 1 #894

merged 8 commits into from
Feb 22, 2024

Conversation

JQGoh
Copy link
Contributor

@JQGoh JQGoh commented Feb 18, 2024

This addresses #890

Main changes

  • Introduce _check_nan() function to validate the input whereby for row records with available_mask=1, we shall raise ValueError if there is NaN value.

Rationale

  • check_nan() is called as part of _prepare_fit(). One main reason we perform the validation check before the execution of TimeSeriesDataset.from_df() is that within the from_df(), it shall use utilsforecast.process_df -> to_numpy() which shall replace NaN values of categorical variable type as -1.0. Hence, we cannot perform validation based on the dataset returned by TimeSeriesDataset.from_df() to flag the NaN values in categorical features.
  • Note that available_mask=1 is automatically assigned as part of TimeSeriesDataset.from_df(). in check_nan(), we follow the same practice and assign available_mask=1 if users do not provide available_mask as one of their input columns.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CLAassistant
Copy link

CLAassistant commented Feb 18, 2024

CLA assistant check
All committers have signed the CLA.

@JQGoh JQGoh marked this pull request as ready for review February 18, 2024 14:43
neuralforecast/core.py Outdated Show resolved Hide resolved
neuralforecast/core.py Outdated Show resolved Hide resolved
@JQGoh
Copy link
Contributor Author

JQGoh commented Feb 19, 2024

@jmoralez Appreciate your review on this. Thanks.

nbs/core.ipynb Show resolved Hide resolved
nbs/core.ipynb Show resolved Hide resolved
nbs/core.ipynb Show resolved Hide resolved
nbs/core.ipynb Show resolved Hide resolved
nbs/core.ipynb Show resolved Hide resolved
@jmoralez
Copy link
Member

@quest-bot loot #890

Copy link

quest-bot bot commented Feb 20, 2024

👋 Hey @jmoralez, you can't loot on a PR you didn't create.

If you think this is a mistake, contact us at help@quine.sh

@jmoralez
Copy link
Member

@JQGoh can you comment the following:

@quest-bot loot #890

@JQGoh
Copy link
Contributor Author

JQGoh commented Feb 21, 2024

@quest-bot loot #890

@quest-bot quest-bot bot mentioned this pull request Feb 21, 2024
@quest-bot quest-bot bot added the ⚔️ Quest Tracks quest-bot quests label Feb 21, 2024
Copy link

quest-bot bot commented Feb 21, 2024

Quest PR submitted! image Quest PR submitted!

@JQGoh, you are attempting to solve the issue and loot this Quest. Will you be successful?


Questions? Check out the docs.

@JQGoh
Copy link
Contributor Author

JQGoh commented Feb 22, 2024

@jmoralez
Thanks for the detailed reviews and suggestions. I have revised accordingly based on your feedback. Please have a second look.

Copy link
Member

@jmoralez jmoralez left a comment

Choose a reason for hiding this comment

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

Thanks a lot! I appreciate the thorough tests

@jmoralez jmoralez merged commit cf9af08 into Nixtla:main Feb 22, 2024
12 checks passed
Copy link

quest-bot bot commented Feb 26, 2024

🧚 @JQGoh congratulations for completing Quest #890

💰 A reward of $100 has been credited to you.

To claim your $100 reward follow the instructions here.

Questions? Check out the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚔️ Quest Tracks quest-bot quests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants