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

Rewrite TestRPmatrix::test_local_exceedance_imp_pass to not rely on loading data #915

Open
riley-brady opened this issue Jul 12, 2024 · 3 comments
Assignees
Labels
accepting pull request Contribute by raising a pull request to resolve this issue! bug task something that needs to be done

Comments

@riley-brady
Copy link

Describe the bug
Following the simple installation guide, the base unit tests currently fail (for a single test). This is a simple roundoff error failure for test_local_exceedance_imp_pass.

FAILED test_impact.py::TestRPmatrix::test_local_exceedance_imp_pass - AssertionError: 2916964966.388227 != 2916964966.388219 within 5 places (8.106231689453125e-06 difference)

To Reproduce
Steps to reproduce the behavior/error:

  1. mamba create -n climada_env -c conda-forge climada
  2. mamba activate climada_env
  3. python -m unittest climada.engine.test.test_impact

Expected behavior
Tests should pass!

Climada Version: 4.1.0

System Information (please complete the following information):

  • Operating system and version: MacOS 14.5. M3 Max chip.
  • Python version: 3.9.18

Additional context

@peanutfun
Copy link
Member

@riley-brady Thank you for your report. We do not observe this issue in our testing pipeline. As the test loads data from Excel and HDF5 files, the issue might be related to particularities of the involved libraries on your system.

Nonetheless, the test can be considered legacy because it does not follow our new unit test standards. I will therefore transform this issue into a task to rewrite the test, such that it does not rely on reading data.

@peanutfun
Copy link
Member

@riley-brady What I forgot to mention: If this is the only failing test in your installation, I think you can safely ignore it for now ✌️

@peanutfun peanutfun changed the title Standard test suite failing on light installation Rewrite TestRPmatrix::test_local_exceedance_imp_pass to not rely on loading data Jul 15, 2024
@peanutfun peanutfun self-assigned this Jul 15, 2024
@peanutfun peanutfun added task something that needs to be done accepting pull request Contribute by raising a pull request to resolve this issue! labels Jul 15, 2024
@riley-brady
Copy link
Author

Thank you! Yes, agreed it does not impact use of climada. I agree that it's likely library-dependent. I tried with and without bottleneck installed, which didn't change anything. But I think it's a roundoff/precision issue related to something like that. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting pull request Contribute by raising a pull request to resolve this issue! bug task something that needs to be done
Projects
None yet
Development

No branches or pull requests

2 participants