User Defined formats considered harmful :-)

[DennisHeimbigner]

I am planning to extend the dispatch table
and was thinking about the consequences for
user defined formats (UDF).

The problem is that the dispatch table format
provided by the user's client code could potentially be different
than that being used by libnetcdf. This means that the # of entries
in the users' table could be more or less (or equal) to the #
of entries in the libnetcdf dispatch table and there is no way
to determine the relative sizes and therefore the relative
compatibility.

I am not sure if this is a problem in practice.
Consider the cases, and assume that the client code is compiled.

1. |client table| < |libnetcdf table| -- presumably the client code
   will not (indirectly or directly) invoke any entries that are
   not in its table. So that should work ok. However if, for some reason,
   the libnetcdf code internally invokes the extra entries, this will
   lead to a memory fault trying to access past the end of the user's table.
2. |client table| > |libnetcdf table| -- presumably this will fail
   at compile time because the client code will reference functions
   not known to libnetcdf, so this should be ok. The only potential
   problem is with cross dispatch table calls where the UDF code
   attempts to call the new entries, but for some dispatch table
   other than the UDF table.

Notes:

1. In case 1, if this bumps the .so number, it should be ok,
   but I do not know if we currently take this into account when
   defining the .so number.
2. The other case where we can get into trouble is with interpretive
   systems such as python where we can, in principle, synthesize
   a call to one of the new entries. In this case we could see various
   memory or segfault errors.
3. I am implementing the ability to define multiple filters per variable.
   One implementation I am considering is the re-implementation of
   the first level filter functions (e.g. nc_def_var_filter)
   to invoke a new function called filter_actions.
   This would fail for case 1 because the nc_def_var_filter wrapper code
   in libdispatch/dvar.c would attempt to access the filter_actions
   entry in the UDF and that would fail.

There are some implications here. In effect, by exporting the dispatch
table format we are not only limiting changes to it, but we must also
limit the implementations of the wrapper functions (like nc_def_var_filter)
so that they always invoke the same dispatch table entries; they cannot
be changed to use new function in the dispatch table.

Adding a version number to the actual dispatch table would
allow graceful failure for some of the above cases, but not all of them.

Bottom line: UDF needs more thought before we release it into the wild.

[edwardhartnett]

UDF is already released into the wild, it is used in PIO in support of netCDF integration and HPC features, and that is a major feature which may already be in use in various models (It is planned to be used in NOAA GFS). As you recall, this is something that was years in the making, and Unidata was involved in every step of planning.

However, I don't believe the UDF version issue is serious.

If there is a mistake by an application like PIO, the result will generally be a segfault. So it is certainly incumbent on PIO to use the correct version table. However, this is neither difficult nor unreasonable. Nor is that something netcdf-c should feel responsible for. The use of version numbers in the dispatch table allows inconsistencies to appear at PIO build time.

There are no limits of how you want to change the dispatch table. We have a version number and you may change it as much as you like. However, historically the list of netCDF functions has been rather stable. I don't see that changing. If there are some changes, we can handle them with versioned dispatch tables.

Nor do I think there is any limitation on what functions are invoked by dispatch table entries. If you call a function in your dispatch layer, you must provide that function. If someone else replaces your function with another, as HDF4 does, then it is up to the HDF4 code to be sure it has all the functions it needs. So what's that about?

[edwardhartnett]

Also, I have no objection to adding a version number to the table itself, but my idea was not that netcdf-c would continue to support old versions.

The version number could be checked against the version number of the dispatch struct that was passed in, and if they don't match, there's an error and the dispatch table is not accepted.

Currently this check can take place in the other library (ex PIO), but adding the version number to the dispatch table itself would allow the check to happen inside netcdf-c as well.

[DennisHeimbigner]

This issue is mostly to noteissues surrounding UDF and possible solutions
or restrictions

Since you control PIO, we could make some mods to the dispatch
table format that you could then implement. So by "in the wild"
I would mean use that is out of our (collectively) control. That does
not exist, as far as I know.

The important restriction I am identifying is that none of the
code in libdispatch should use dispatch entries that are not
consistent with the function. That is, if I add a function to the API,
it must not later be modified to use newly introduced dispatch entries.
It must continue to only use dispatch entries that existed when it
was added to the API.

Intra dispatch table calls are ok. It is inter-dispatch calls that may be a problem. For example, libdap2 calls libsrc functions to implement many netcdf-3 specific operations. The UDF code would, ideally,
to a version check to ensure this was ok.

In retrospect, t was probably a mistake on my part to merge
netcdf API calls into a single function: i.e inq_var_all(). But
that is water under the bridge.

[edwardhartnett]

But wouldn't any missing inter-dispatch functions be found at compile time? That is, if my UDF dispatch layer calls some function that is not in the library, it will not build.

For example: when using PIO, first netcdf-c is built, and then PIO is built. If PIO calls a function not in netcdf-c, then PIO will not link.

If you mean shared objects, then netcdf-c should already be setting version-info correctly. Any release of netcdf-c which removed an internal function will reflect that in the so-number.

[DennisHeimbigner]

Ed convinced me that I should not change the signature, so I will
add a new function nc_inq_var_filterid instead.

[edwardhartnett]

Did you mean you are going to add nc_inq_var_filtersids()?

[edwardhartnett]

OK, I think we discussed some of the potential pitfalls of misuse of the user-defined format feature, and those are good to keep in mind. It is an advanced feature, intended for advanced users and libraries like PIO, which have a legitimate need to know a lot about netCDF.

In terms of making is safer, Dennis has proposed a version number, which we already have. But what I didn't get, but he probably did, was that we should put this version number in the table definition. So that way, I can encode version "2" into my PIO table. If netcdf-c later changes to version 3, when my table is submitted, nc_def_user_format() can give an error. Then we prevent a lot of confusion.

So I propose to add the version number to the dispatch table. Each dispatch table will have to explicitly define what version it is, and netcdf-c will check that the version matches its internal requirement.

Note that there is no idea of attempting to handle more than one version in the netcdf-c library at a time. Old versions will be rejected with an error when nc_def_user_format() is called. It is up to applications like PIO to keep up, or understand what versions of netcdf-c it is compatible with.

@DennisHeimbigner does this seem agreeable?

[DennisHeimbigner]

Yes.

The new filter functions will be:

• nc_inq_var_filterids - return the set of ids of all filters associated with a var
• nc_inq_var_filterid - return the parameters associated with a given filter id for
  a given var. Might change the name to nc_inq_var_filterarameters
  or something similar

[edwardhartnett]

Usually when we say inq_somethingid in the API, we mean that an ID will be returned, like nc_inq_varid, nc_inq_dimid, nc_inq_attid.

Seems like you already have an nc_inq_var_filter that returns the parameters, given the filter ID. So it seems that simple adding nc_inq_var_filterids() would solve your problems by returning an array of filter IDs, which could then be run, one by one, through nc_inq_var_filter() to get all the parameters.

This is very similar to how multiple unlimited dims are handled in netcdf-4, so I think it would fit naturally into the API.