Skip to content

Fix feature coordinate handling in WeatherDataset#558

Closed
2024itb047samata wants to merge 1 commit intomllam:mainfrom
2024itb047samata:fix-weather-feature-clean
Closed

Fix feature coordinate handling in WeatherDataset#558
2024itb047samata wants to merge 1 commit intomllam:mainfrom
2024itb047samata:fix-weather-feature-clean

Conversation

@2024itb047samata
Copy link
Copy Markdown

Describe your changes

This PR fixes incorrect feature coordinate handling in WeatherDataset.

Previously, the implementation hardcoded access to state_feature, which caused failures for other dataset categories such as forcing and static.

This change dynamically selects the correct coordinate using f"{category}_feature", accesses it via da_datastore_state.coords[feature_key], and adds validation to ensure the coordinate exists. It also improves error messages for easier debugging.

Issue Link

Closes #536

Type of change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📖 Documentation (Addition or improvements to documentation)

Checklist before requesting a review

  • My branch is up-to-date with the target branch - if not update your fork with the changes from the target branch (use pull with --rebase option if possible).
  • I have performed a self-review of my code
  • For any new/modified functions/classes I have added docstrings that clearly describe its purpose, expected inputs and returned values
  • I have placed in-line comments to clarify the intent of any hard-to-understand passages of my code
  • I have updated the README to cover introduced code changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have given the PR a name that clearly describes the change, written in imperative form (context).
  • I have requested a reviewer and an assignee (assignee is responsible for merging). This applies only if you have write access to the repo, otherwise feel free to tag a maintainer to add a reviewer and assignee.

Checklist for reviewers

  • the code is readable
  • the code is well tested
  • the code is documented (including return types and parameters)
  • the code is easy to maintain

Author checklist after completed review

  • I have added a line to the CHANGELOG describing this change, in a section
    reflecting type of change (add section where missing):
    • added: when you have added new functionality
    • changed: when default behaviour of the code has been changed
    • fixes: when your contribution fixes a bug
    • maintenance: when your contribution is relates to repo maintenance, e.g. CI/CD or documentation

Checklist for assignee

  • PR is up to date with the base branch
  • the tests pass
  • (if the PR is not just maintenance/bugfix) the PR is assigned to the next milestone. If it is not, propose it for a future milestone.
  • author has added an entry to the changelog (and designated the change as added, changed, fixed or maintenance)
  • Once the PR is ready to be merged, squash commits and merge the PR.

@sadamov
Copy link
Copy Markdown
Collaborator

sadamov commented Mar 31, 2026

#556 tackles this issue already. PLEASE @2024itb047samata stop submitting PRs without checking for dupes

@sadamov sadamov closed this Mar 31, 2026
@2024itb047samata
Copy link
Copy Markdown
Author

Hi @sadamov,

I noticed that #556 is still open, which is why I worked on this issue to provide a minimal and focused alternative fix for the feature coordinate handling.

I understand the concern about overlapping work, and I’ll be more careful to coordinate and check active PRs beforehand.

That said, I would really appreciate more specific guidance on how you’d prefer contributors to proceed in such cases (e.g., whether to wait, collaborate on an existing PR, or propose alternatives). That would help avoid similar situations going forward.

Happy to adjust or close this PR if needed.

@sadamov
Copy link
Copy Markdown
Collaborator

sadamov commented Mar 31, 2026

Hi @2024itb047samata, are you already a part of our slack channel kutt.to/mllam? Joel posted this message some weeks ago about contributing to neural-lam: https://ml-lam.slack.com/archives/C0AFPLZ682X/p1772895022445439

I think your questions should be answered there. It is however a totally fair criticism that we don't yet have a proper CONTRIBUTING.md file, which is being worked on in #407. I hope we get this merged soon and then things should become even clearer.

i everyone interested in GSoC 2026! We have made some further clarifications to our GSoC contributor guidelines in order to make the preparation as smooth as possible for everyone, and make it clear what we look for in contributors :smile: You can read through the full guidelines here: https://github.com/mllam/neural-lam/wiki/GSoC-ideas, but I also summarize the most important points below:

Guidelines for code contributions

For selecting GSoC proposals, we do not care about how many PRs you have submitted/merged. There is no hard requirement on having previously contributed code. We are looking for genuine engagement and interest, and quality over quantity.
Try to avoid opening duplicate PRs, instead helping out in the existing PR by proposing improvements.
If you want to start working on fixing an open issue, make sure to comment on the issue and request to be assigned to it. That way we can make sure only one person is working on each issue. Don't open a PR before there is some agreement in the issue that the change is good and that the proposed solution is suitable.
If we do not see any progress within 7 days of being assigned an issue (e.g. by opening a PR and actively engage in the conversation) we might re-assign the issue again.
Make sure to always use our PR template and follow the instructions in there.


Writing the proposal

We have made some modifications to the proposal template, the new one now being located here: https://github.com/mllam/neural-lam/wiki/GSoC-proposal-template
If you have already started working on your proposal there is no requirement to switch to this new one. But you can note that the differences are very small.

To get feedback on a proposal draft, please submit a link to a google doc with comment permissions in [this form](https://docs.google.com/forms/d/e/1FAIpQLSciGrfW9Q-NptrD5d4GREqQdsV___1WOzMBywKj8NCbM901fA/viewform?usp=sharing&ouid=106814839361720537667). This allows us to keep better track on what drafts have got feedback from mentors yet.
For questions about the projects, please use the [#gsoc-project1](https://ml-lam.slack.com/archives/C0AG3GJJH9T)/[#gsoc-project2](https://ml-lam.slack.com/archives/C0AG4T7LJBG) /[#gsoc-project3](https://ml-lam.slack.com/archives/C0AGKSS4LAV)/[#gsoc-project4](https://ml-lam.slack.com/archives/C0AG3GVATHT) channels on slack rather than directly messaging mentors.

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.

Incorrect Feature Coordinate Handling in WeatherDataset

2 participants