search

FlowModelingControl / flowtorch

flowTorch - a Python library for analysis and reduced-order modeling of fluid flows
GNU General Public License v3.0
131 stars 45 forks source link

potential pytest issues #26

Closed akaptano closed 2 years ago

akaptano commented 2 years ago

Hello again. I am now basically done reviewing the pytest tests + non-code documentation + paper, and only the documentation within the code and reproducing/suggesting improvements for the examples remains. I will open an issue for these separately. I am happy to see some good pytest tests. These mostly work for me after downloading the big dataset. However, I still have the following issues:

  1. Failure in analysis/test_psp_explorer.py because it looks like the test file is hard-coded on your system, "FileNotFoundError: Could not find file /home/andre/Downloads/input/FOR/iPSP_reference_may_2021/0226.hdf5"

  2. I get a mysterious segmentation fault when using pytest on flowtorch/rom/test_cnm.py, although it appears the tests are sometimes completing correctly. The cnm_cylinder.ipynb jupyter notebook also crashes for me when the CNM function is called. See below:

    
flowtorch/rom/test_cnm.py: 16 tests with warnings
/Users/alankaptanoglu/flowtorch/flowtorch/rom/utils.py:72: DeprecationWarning: `np.bool` is a deprecated alias for the builtin `bool`. Use `bool` by itself, which is identical in behavior, to silence this warning. If you specifically wanted the numpy scalar type, use `np.bool_` here.
is_different = np.diff(sequence).astype(np.bool)

-- Docs: https://docs.pytest.org/en/latest/warnings.html ================================================== 10 passed, 18 warnings in 5.54s ================================================== Fatal Python error: Segmentation fault: 11 



3. (Optional) Ideally, it would be nice if the pytest tests were not dependent on first downloading a big 2.5 GB file. I think many of these tests should still work on synthetic datasets of smaller size.
AndreWeiner commented 2 years ago

Hi Alan, thanks again :-) 1) stupid, fixed - thanks 2) The warning seems to be related to the netCDF4 library and was apparently fixed here; see also this issue. I couldn't reproduce the warning with netCDF4 version 1.5.6. I guess an update might fix the warning. 3) Unfortunately, I have to deal with some data formats coming from external sources, and I only have limited or no access to the tools that generated the data. Creating synthetic replacements for these data would be extremely cumbersome (I am not really happy with that situation). As a compromise, I created a reduced version of the full dataset, which is only about 340MB in size but still works with the same unit tests as the full dataset. The instructions in the README are updated accordingly.

If you are happy with these fixes, feel free to close this issue. Best, Andre