schism-dev / pyschism

Python interface for handling the SCHISM model.
https://schism-dev.github.io/schism/master/getting-started/pre-processing-with-pyschism/overview.html
Apache License 2.0
23 stars 18 forks source link

`ElevIc` no longer is a `Gr3Field` type #81

Closed SorooshMani-NOAA closed 1 year ago

SorooshMani-NOAA commented 1 year ago

Since the change in https://github.com/schism-dev/pyschism/commit/5b5ded0d98ffb0776a1a99538cb33f99d1ffa1a8, the ElevIc object is no longer a Gr3Field type and hence cannot be as easily manipulated, e.g. using constant, add_region, etc. For example if someone wants to use different initial elevation values that the default.

Is there any reason why ElevIc is no longer inherting IcField or Gr3Field?

This is related to https://github.com/schism-dev/schism/issues/105 discussed with @cuill

cuill commented 1 year ago

@SorooshMani-NOAA This was fixed in a separate branch (https://github.com/schism-dev/pyschism/tree/feature/ElevIc), can you test it and let me know if it is working?

SorooshMani-NOAA commented 1 year ago

@cuill I'm testing, thanks! Just one question: is elev.ic file positive up or down?

cuill commented 1 year ago

@SorooshMani-NOAA it is positive up.

SorooshMani-NOAA commented 1 year ago

@cuill, the update in feature/ElevIc branch code works fine except that it needs to return an ElevIc object instead of Hgrid. To fix that instead of copying the hgrid, you can use it first to create a constant ElevIc object first and then update its values.

  def default(cls, hgrid, offset=0.1):
-     obj = hgrid.copy()
-     obj.values[:] = -np.maximum(0.0, hgrid.values - offset)
+     obj = cls.constant(hgrid, 0.0)
+     obj.values[:] = np.maximum(0.0, hgrid.values - offset)
      return obj

Note that because of the way pyschism handles positive up vs down between files vs various Gr3 objects, you need to remove the minus sign as well (as above).

Since I cannot push to schism-dev directly please make these changes. I did a quick test and it looks to be working fine. Thanks!

cuill commented 1 year ago

@SorooshMani-NOAA The code was changed and a new release (v0.1.12) was published.

SorooshMani-NOAA commented 1 year ago

I'll test by running a tidal setup and close the ticket if successful, thanks!