OpenDrift / opendrift

Open source framework for ocean trajectory modelling
https://opendrift.github.io
GNU General Public License v2.0
231 stars 113 forks source link

small fix #1265

Closed kthyng closed 3 months ago

kthyng commented 3 months ago

assertion line for z_rho in ROMS reader does not ignore nans. But also it is probably too much to have that assertion so fixed to ignore nans but also changed to a warning.

kthyng commented 3 months ago

@knutfrode sorry a fix already!

knutfrode commented 3 months ago

All tests are passing, however, this example script is failing: https://opendrift.github.io/gallery/example_vertical_mixing.html

    ../../examples/example_vertical_mixing.py failed leaving traceback:

    Traceback (most recent call last):
      File "/root/project/examples/example_vertical_mixing.py", line 51, in <module>
        depths = o.vertical_mixing(store_depths=True)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/root/project/opendrift/models/oceandrift.py", line 502, in vertical_mixing
        Zmin = -1.*(self.environment.sea_floor_depth_below_sea_level + self.environment.sea_surface_height + Dcrit)
                                                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/opt/conda/envs/opendrift/lib/python3.11/site-packages/numpy/core/records.py", line 455, in __getattribute__
        raise AttributeError("recarray has no attribute %s" % attr) from e
    AttributeError: recarray has no attribute sea_surface_height

This is a bit special example, that runs the mixing loop without running the main simulation. Anyway, it would be best it it can be made to pass. And also it might be a warning of something that can crash also in other contexts.

kthyng commented 3 months ago

@knutfrode Ah I don't know why that is coming up now and not before (the small fix I made shouldn't have impacted that?), but I had to fix an issue like that in a test that ran for only vertical_mixing. I'll try to go find it. The same fix from that test should fix the issue in the example, but how should I run the examples locally?

knutfrode commented 3 months ago

You can simply run the examples as standalone scripts

knutfrode commented 3 months ago

The examples sometimes (as now) detects things that are not covered by the tests

kthyng commented 3 months ago

No this was previously caught by a test too, not sure why the example didn't catch it before. Anyway I didn't run this locally but I applied the same fix from a test so hopefully it works.

knutfrode commented 3 months ago

Now the same example failed with

    ../../examples/example_vertical_mixing.py failed leaving traceback:

    Traceback (most recent call last):
      File "/root/project/examples/example_vertical_mixing.py", line 41, in <module>
        o.environment = np.array(np.ones(N, 0)*sea_floor_depth,
                                 ^^^^^^^^^^^^^
      File "/opt/conda/envs/opendrift/lib/python3.11/site-packages/numpy/core/numeric.py", line 191, in ones
        a = empty(shape, dtype, order)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^
    TypeError: Cannot interpret '0' as a data type
knutfrode commented 3 months ago

The last commit seems to fix the problem (just the HYCOM thredds server hanging in some of the example scripts). Thus merging now.

kthyng commented 3 months ago

@knutfrode Ah, thank you! Sorry my blind fix didn't actually fix it.