ec-jrc / pyposeidon Goto Github PK

When the grid crosses the International line we get

ABORT: Negative elem. areas; check nonfatal_*

Corresponding checks within test_schism_total.py fail.

There was no problem with SCHISM < v5.7 but then something changed.

This applies to the update argument specifically.

The model.result() function does output merging for all outputs. There should be an option to give a range. This will be useful in intermitted runs.

This mainly includes the grid generation procedure. Also make sure the abstraction works for both solvers.

I have resolved temporarily an issue with jigsaw and the in-house ocean coastlines. It works with cbuffer now so it might be the quality of the coastlines or something in the configuration.

Strangely the same cbuffer with the gmsh option doesn't pass the schism validation.

Latest version of jigsaw needs more testing.

cfgrib is pinned to 0.9.8.5 for now. See how to move pass that.
Follow pydata/xarray#5150

Setting use_threads: True by default works with openmpi but not mpich.

We need either to identify which flavour of mpi is installed or do a get_value() within schism.py so that the user specifies use_threads: False when using mpich.

The server that hosts the Natural Earth features seems to be misbehaving. I 've noticed both slow responses and plain failures (example).

The files we are trying to download are hosted at: https://naciscdn.org/naturalearth/110m/physical/ne_110m_land.zip
There is an open issue for the server here: nvkelso/natural-earth-vector#581 (comment)
The issue mentions that the data we need can be downloaded from:

• https://naturalearth.s3.amazonaws.com/110m_physical/ne_110m_land.zip
• https://naturalearth.s3.amazonaws.com/50m_physical/ne_50m_land.zip
• https://naturalearth.s3.amazonaws.com/10m_physical/ne_10m_land.zip

The data are a few MBs only.

There are several options:

1. Do nothing and wait until the server gets restored.
2. "Inline" them in the project, i.e. git add the files in tests/data.
3. host them ourselves on e.g. dropbox

Options 2 and 3 will probably allow us to stop having a hard dependency on cartopy, too (it will become an optional dependency just for visualization).

@brey what do you think?

We currently use cartopy 0.18. Installing cartopy 0.18 with pip requires that you already have numpy installed in the virtualenv. This is not a huge issue, but it requires that you do:

pip install numpy
pip install pyposeidon

instead of a plain

pip install pyposeidon

In 0.19 this has been fixed with SciTools/cartopy#1681
Nevertheless, before bumping up the version we should first check if everything works fine with 0.19

@brey since you are more familiar with the visualization functionality, could you check this please? 0.19 is already on conda-forge.

We should setup:

• pre-commit for local development (link)
• https://pre-commit.ci. I am not sure if I have the permissions to set this one up. We might need to ask the ec-jrc admin to authorize it

pplot.mesh() defaults suffer from "over-plotting". For example:

Should we add some default values for lineweight and markersize?

Adjust for touches as opposed to crosses the coastline.

Replace meteo_engine attribute within meteo.py with an open_dataset function similar to dem.py

This includes docstrings.

The env.yml works only for Linux since the libglu is not available for OS X nor is needed.

The best solution is the implementation of conda/conda#8089.

Check why the fix.py function produces NaNs in the European window.

The read_msh function only returns [nodes,edges,tria]. Expand that to return a complete xarray object.

pyposeidon/utils/ needs to be cleaned up. There is code that is not being used anywhere, code that cannot be imported due to missing dependencies etc. If the snippets are useful we could transfer them to a wiki

I transferred here the comments by @vvoukouvalas

https://pyposeidon-panos.readthedocs.io/en/latest/structure.html

on the functionality of dem: is it only the bathymetric data or also topobathy? you have done a lot of work also on separating the initial dry/wet area etc

https://pyposeidon-panos.readthedocs.io/en/latest/meteo.html

small typo at the section URL of "As an ..."

https://pyposeidon-panos.readthedocs.io/en/latest/dem.html

first line same as above, is it only for bathymetric data?

Coastlines section, small typo : "is usually lower than the one of the available..."

Coastlines section, "Since the coastlines are the boundary for the mesh generation.." : is it possible to use e.g. the 10m land contour (assuming that the user provides a proper one without overlaps etc) to make a model for inundation studies?

small typo, maybe something like "Generally, whatever format geopandas reads should work."

small typo, "... bathymetric data on the x,y nodes can be provided as"

https://pyposeidon-panos.readthedocs.io/en/latest/boundary.html

Buffering section, what are the units of cbuffer?

https://pyposeidon-panos.readthedocs.io/en/latest/mesh.html

it is called GSHHG dataset not GSHHS

"... is an xarray Dataset. Thus," the tip below interrupts the flow

https://pyposeidon-panos.readthedocs.io/en/latest/model.html

"The procedure usually includes providing several info :", please reformulate.
" ... and reproduced it with"
" ... there might be a model which has been created by other means. This model can be ... ",

https://pyposeidon-panos.readthedocs.io/en/latest/utils.html

line 5: " ... there is a number of available functions

"Having the new 2D topology, computing the corresponding subset of any variable in a Dataset becomes:" : what is it computed here?

https://pyposeidon-panos.readthedocs.io/en/latest/development.html

"... and use it to create a new conda environment. E.g. The following" : please reformulate it.

" ... for testing pyposeidon are substantial in size", please give an indication of the size.

https://pyposeidon-panos.readthedocs.io/en/latest/api.html

section pyposeidon.model: correct to hydrodynamic models

sections schism and d3d "It controls the creation, output & execution" : creation, execution & output

sections mjigsaw and mgmsh: is there any way to change the parameters of these libraries?

Locally during development one can use

python -m pip install -e .

to have pyposeidon dynamically installed in the local conda env.

However when a pyproject.toml is present the command above starts installing everything with pip.

Is there a way to mask the presence of pyproject.toml without renaming the file?

We currently pin jigsaw to 0.9.12. This is not currently causing problems, but eventually might cause conda conficts.

The reason we pin the version is that some of the parameters we currently use are causing a segmentation fault in jigsaw 0.9.12+. I tried to update the parameters and the tests are passing but I guess we should check the output in more detail.

dengwirda/jigsaw#47

ShapelyDeprecationWarning: __len__ for multi-part geometries is deprecated and will be removed in Shapely 2.0. Check the length of the geoms property instead to get the number of parts of a multi-part geometry.

Test [min,max] option. Expand to global.

Follow pydap/pydap#211 to check when pydap > 3.2.2 will solve the problem.

Avoid hardcoded schism references. This should make working with pyposeidon output easier, simplify the visualization accessors etc.

On WSL, after creating a new environment with

conda create -n pyPoseidon python=3.8
conda activate pyPoseidon

and attempting to install from conda:

conda install -c gbrey pyposeidon

I get the following output:

Collecting package metadata (current_repodata.json): done
Solving environment: failed with initial frozen solve. Retrying with flexible solve.
Solving environment: failed with repodata from current_repodata.json, will retry with next repodata source.
Collecting package metadata (repodata.json): done
Solving environment: failed with initial frozen solve. Retrying with flexible solve.
Solving environment: |
Found conflicts! Looking for incompatible packages.
This can take several minutes.  Press CTRL-C to abort.
failed

UnsatisfiableError: The following specifications were found to be incompatible with each other:

Output in format: Requested package -> Available versions

No packages are displayed.

ffmpeg is currently used by :

$ ag ffmpeg --py
pyposeidon/utils/mplot.py
24:ffmpeg = sys.exec_prefix + "/bin/ffmpeg"
25:os.environ["FFMPEG_BINARY"] = ffmpeg

pyposeidon/utils/pplot.py
26:ffmpeg = sys.exec_prefix + "/bin/ffmpeg"
27:os.environ["FFMPEG_BINARY"] = ffmpeg

Running the code/tests throws these warning.

  /home/amigo/repo/pyPoseidon/grid.py:163: PendingDeprecationWarning: dropping variables using `drop` will be deprecated; using drop_vars is encouraged.
    grid = grid.drop('nSCHISM_hgrid_node')

Removing them would be nice as they only add noise to the test output.

We have a new warning

tests/test_boundary.py::test_isodem[local geotiff-window0-input1]
  /home/panos/Prog/poseidon/pyPoseidon/pyposeidon/tools.py:105: DeprecationWarning: open_rasterio is Deprecated in favor of rioxarray. For information about transitioning, see: https://corteva.github.io/rioxarray/stable/getting_started/getting_started.html
    data_array = xr.open_rasterio(source, parse_coordinates=True, **kwargs)

Either Travis or Azure (or both).

Try to parallelize the output merging of SCHISM's data.
Definitely look again into the code which is quite convoluted.

Pass kwargs in read_mesh calls below

Manage internally the failure of both subprocesses.

Use the tool module to launch the D3D job as well because now it fails with openmpi.

pygeos is now supported by Geopandas. Check whether we can simplify the code by implementing only geopandas.

I am trying to create the lock files using both delft and schism. It seems to be somewhat challenging. Most of the problems seem to be coming from the incorporation of the delft package.

Note: all this has been tried on Linux. On Mac, conda might be able to resolve these, but I haven't tried them

1. First of all, we need to bump up the gmsh minimum version to >4.8.4. The is the MRE (Minimum Reproducible Example):

$ mamba create --dry-run --name whatever --override-channels --channel conda-forge \
  'gbrey::delft3d4=65936*=mpi_mpich*' \
  'eccodes>2.20'\
  'gmsh=4.8.4'

[...]

Encountered problems while solving:
  - package netcdf-fortran-4.5.4-mpi_mpich_h1364a43_0 requires hdf5 >=1.12.1,<1.12.2.0a0 mpi_mpich_*, but none of the providers can be installed

After we bump the gmsh version, the conflict is resolved:

$ mamba create --dry-run --name whatever --override-channels --channel conda-forge \
  'gbrey::delft3d4=65936*=mpi_mpich*' \
  'eccodes>2.20'\
  'gmsh>4.8.4'

2. Adding proj<8 to the mix seems to also cause conflicts...

$ mamba create --dry-run --name whatever --override-channels --channel conda-forge \
  'gbrey::delft3d4=65936*=mpi_mpich*' \
  'eccodes>2.20'\
  'gmsh>4.8.4' \
  'proj<8'

[...]

Encountered problems while solving:
  - package gmsh-4.9.3-h31d5161_1 requires occt >=7.6.0,<7.7.0a0, but none of the providers can be installed

Therefore, we need to bump proj, too. The reason we have pinned proj<8 is various issues we had with cartopy, performance etc. Bumping proj>8 does seem to resolve the conflicts but we need to test again all the visualization stuff that they are working as expected.

3. now if we try to add geos and its python wrappers in the mix we get another conflict:

mamba create --dry-run --name whatever --override-channels --channel conda-forge \
  'gbrey::delft3d4=65936*=mpi_mpich*' \
  'eccodes>2.20'\
  'gmsh>4.8.4' \
  'proj>=8' \
  geos \
  'pygeos>=0.10' \
  'shapely<1.8' \
  geopandas
  
[...]

Encountered problems while solving:
  - package gmsh-4.9.3-h31d5161_1 requires occt >=7.6.0,<7.7.0a0, but none of the providers can be installed

Which means that we need to bump up shapely to 1.8, too. I don't remember now the exact details but I do remember that I had trouble when I tried to do that some weeks back.

4. nevertheless, regardless of these versions, there is also at least one more conflict which seems to be non-resolvable whatsoever. mayavi!:

mamba create --dry-run --name whatever --override-channels --channel conda-forge \
  'gbrey::delft3d4=65936*=mpi_mpich*' \
  'eccodes>2.20'\
  'gmsh>4.8.4' \
  'proj>8' \
  mayavi

[...]

Encountered problems while solving:
  - package eccodes-2.21.0-ha0e6eb6_0 requires libnetcdf >=4.7.4,<4.7.5.0a0, but none of the providers can be installed

I tried to remove the eccodes constraint, but the packages don't get resolved even after we remove it:

$ mamba create --dry-run --name whatever --override-channels --channel conda-forge \
  'gbrey::delft3d4=65936*=mpi_mpich*' \
  'eccodes'\     
  'gmsh>4.8.4' \
  'proj>8' \
  mayavi

[...]

Encountered problems while solving:
  - package netcdf-fortran-4.5.4-mpi_mpich_h1364a43_0 requires libnetcdf >=4.8.1,<4.8.2.0a0 mpi_mpich_*, but none of the providers can be installed

I am running on a laptop with an AMD Ryzen 4800H processor. This means that I am using an 8 cores/16 threads machine.

When I try to execute launchshism.sh I get the following error:

--------------------------------------------------------------------------
There are not enough slots available in the system to satisfy the 15
slots that were requested by the application:

  schism

Either request fewer slots for your application, or make more slots
available for use.

A "slot" is the Open MPI term for an allocatable unit where we can
launch a process.  The number of slots available are defined by the
environment in which Open MPI processes are run:

  1. Hostfile, via "slots=N" clauses (N defaults to number of
     processor cores if not provided)
  2. The --host command line parameter, via a ":N" suffix on the
     hostname (N defaults to 1 if not provided)
  3. Resource manager (e.g., SLURM, PBS/Torque, LSF, etc.)
  4. If none of a hostfile, the --host command line parameter, or an
     RM is present, Open MPI defaults to the number of processor cores

In all the above cases, if you want Open MPI to default to the number
of hardware threads instead of the number of processor cores, use the
--use-hwthread-cpus option.

Alternatively, you can use the --oversubscribe option to ignore the
number of available slots when deciding the number of processes to
launch.
--------------------------------------------------------------------------

My understanding is that the number of cores that is being passed to the CPU is determined by using multiprocessing.cpu_count().

Side note: according to the docs, len(os.sched_getaffinity(0)) is probably a better choice in the general case, but that's not important here. Both methods return 16 on my setup.

Anyhow, even though 15 cores/threads are indeed available, nevertheless, mpirun throws the error I pasted. I googled a bit and I found that there are two options to resolve this:

1. Use mpirun --oversubscribe ... which in case you do oversubscribe the machine, might degrade performance
2. Use mpirun --use-hwthread-cpus ... which AFAI can tell allows mpirun to use CPU threads as cores.

Option 2 feels like the better choice, but I don't have much experience with MPI so I can't really say if there are any downsides.

The users should be able to specify the ax where the figure will be plotted. This is useful for creating complex subplots.

https://github.com/ec-jrc/pyPoseidon/blob/06cdcaa21f839f8531e067e9c633cc292fdbe30f/pyposeidon/schism.py#L1184for

I think the code should look like this:

    for key in tfs[0].variables:
        if 'nSCHISM_hgrid_face' in tfs[0][key].dims :
            r = combine_(key,tfs,melems,'nSCHISM_hgrid_face')
            el.append(r)
        elif 'nSCHISM_hgrid_node' in tfs[0][key].dims :
            r = combine_(key,tfs,mnodes,'nSCHISM_hgrid_node')
            node.append(r)
        elif 'nSCHISM_hgrid_edge' in tfs[0][key].dims :
            r = combine_(key,tfs,msides,'nSCHISM_hgrid_edge')
            side.append(r)
        elif len(tfs[0][key].dims) == 1:
            single.append(tfs[0][key])