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

Minor update to depth- and time-dependent unit testing #65

Merged
merged 13 commits into from
Dec 5, 2024

Conversation

michaeldenes
Copy link
Member

There was an issue with the unit testing, caused by a recent update to parcels. The testing data uses only 5 levels, down to 4.7m depth. Particles would cause an out of bounds error in such a case, and be deleted. Updating the density of the particles lets them still sink, but not as quickly as before.

…o parcels. The testing data uses only 5 levels, down to 4.7m depth. Particles would cause an out of bounds error in such a case, and be deleted. Updating the density of the particles lets them still sink, but not as quickly as before.
@michaeldenes
Copy link
Member Author

Additionally, there was some missing wind data (updated in 2e48261, which includes a few more days of data) causing a time-interpolation error. I'm not sure how these passed in the past, unless time_extrapolation was set to true by default, and no longer?

…, and our tests only run on two days of data, I have named the second week data 2020-01-05, even though the data is defined for 2020-01-11. Parcels will correctly interpolate the data temporally, but this is just to ensure the data is read by the constructors.
@michaeldenes michaeldenes changed the title Minor update to depth-dependent unit testing Minor update to depth- and time-dependent unit testing Dec 4, 2024
@erikvansebille
Copy link
Member

Additionally, there was some missing wind data (updated in 2e48261, which includes a few more days of data) causing a time-interpolation error. I'm not sure how these passed in the past, unless time_extrapolation was set to true by default, and no longer?

The change we made in v3.0.5 that could have impacted this is that the particle.state is the maximum of any statuscode returned by the Field interpolations during an iteration, whereas before v3.0.5 it was the latest statuscode. So if in your list of Kernels the last Field interpolation returned a SUCCESS, then previous errors were not stored on the particle.state. See #1645 for the details of this change

@michaeldenes
Copy link
Member Author

@erikvansebille, this makes a lot of sense, and why I didn't pickup any of these data related issues earlier! Thanks for linking this!

michaeldenes and others added 5 commits December 5, 2024 12:04
…errors when particles are kicked below the test dataset.
…ce the VerticalMixing kernel uses a 0.5m distance in the finite-differencing scheme. Also only running the test for 1d rather than 2.
@michaeldenes
Copy link
Member Author

@VeckoTheGecko would you mind helping me look at why the readthedocs build fails?

@VeckoTheGecko
Copy link
Contributor

VeckoTheGecko commented Dec 5, 2024

Its to do with Python 3.13. I'll put up a PR actually I'll just push here

@michaeldenes
Copy link
Member Author

Thank you! Glad I asked, I wouldn't have picked up on that!

@VeckoTheGecko
Copy link
Contributor

VeckoTheGecko commented Dec 5, 2024

Done. Can you check the site is as you expect once its rendered? (i.e., no visual differences). It looked fine when I did it, so should be fine

@michaeldenes
Copy link
Member Author

@VeckoTheGecko looks good, thanks mate!

@michaeldenes michaeldenes merged commit aaf0871 into main Dec 5, 2024
5 checks passed
@michaeldenes michaeldenes deleted the fix_unit_test_failures branch December 5, 2024 12:31
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