The fundamental design principle of Cassandra is that components don't know too much about each other, so the docstring should be specifying what the component does, not how we expect it to be used. There could well be situations in which flegen uses the HectorStubComponent or some other component (such as a full Hector component) for its Tgav input. Conversely, there might be other components that use the data from this component from time to time.

Either way, it's for the user to decide what component they use for Tgav, or for any other capability, so that information doesn't belong in the component documentation.

I like that we finally have a concrete example of having alternate implementations of the same capability. With the addition of this component, any time global temperature is required, users have a choice of whether they use the Hector stub or this one. Swapping out the source of the Tgav data is as simple as changing the configuration file.

One thing that I did just notice here is that the HectorStub component provides two other capabilities, atm-co2 and Ftot. This component provides only Tgav, which creates a potential problem if you want to sub out the source of Tgav but keep the Hector atm-co2 and Ftot outputs (perhaps for use in some other component). I think what we need is a configuration option to direct a component not to export one or more of its capabilities. I'll create an issue for that.

I found this section of the documentation a little confusing. This sentence says there are two capabilities, but I had to read it a couple of times to figure out what they were. It looks like each component does this documentation a slightly different way, which is unfortunate, but I think the Xanthos component comes the closest to what we want. Here is the corresponding section from its documentation:

    params:
          config_file    - Path to Xanthos config file
          OutputNameStr  - Name for the directory to create for Xanthos outputs

    Capability dependencies (all optional):
           gridded_pr  - List of gridded monthly precipitation by grid cell
          gridded_tas  - List of gridded monthly temperature by grid cell
     gridded_pr_coord  - Matrix of lat/lon coordinates for the precip grid cells
    gridded_tas_coord  - Matrix of lat/lon coordinates for the tas grid cellss

    results:
       gridded_runoff  - Capability 'gridded_runoff', a list of runoff matrices,
                         (gridcells x timestep) with the units and aggregation
                         level specified in the Xanthos config file

The only changes I would recommend to this are:

1. Change "results" to "capabilities", since that's what they are.
2. Reverse the order, so that capabilities come first, dependencies second, and parameters third.

That way, the first thing users see is a description of what the component does, which is the main thing they need to decide whether they want to use the component. The next is what other capabilities have to be in the system, which they need in order to plan their configuration, and the last thing is the parameters, which they need to actually write the configuration.

I take it the tgav_metadata is the second capability offered by the component? If so, then I really don't like the idea of making it a separate capability from the capability that the metadata describes. Doing it that way puts too much onus on the user code to separately request the required data, and probably to produce the metadata as well.

Could we add the metadata as an attribute to the object that holds the data? That way, a component requesting data always gets the metadata along with it, even if the component is unaware that the metadata even exists. We could also have the ComponentBase.addResults method compile the metadata from the component's configuration input, so that components that are unaware of metadata. We could even establish a convention that configuration parameters beginning with X- are ignored by the rest of the argument parsing and simply copied into metadata. That way, a user writing a configuration file could arrange for any information they thought relevant to be recorded in the metadata without having to modify any of the component code.

As I think about it more, having the metadata handled by the framework instead of in the component code has another benefit, viz., that we can be certain of getting standardized metadata, not just whatever the component author thought to put in.

I think it would be a cleaner design to just write a function in the fldgen package to extract the information we want here (i.e., global temperature time series) and return it as a vector. Then this component would have only to call that function and store the return value (If I remember right, rpy2 automatically converts R vectors to numpy 1-D arrays).

I see two benefits to doing it that way. First, the added functionality in fldgen would probably be useful to fldgen users. Second, the code to handle the fldgen structures would be a lot simpler in R, where you can handle the relevant data structure natively, so the overall code complexity would be a lot lower.

@crvernon, I was just discussing this with @FeralFlows, and it looks like we may be running into memory exhaustion. If so, then those problems are surely exacerbated by storing the entire rds file contents in a member variable, since that ensures that the data can't be gc'd until the component goes out of scope, which is basically the whole run. As far as I can tell, it doesn't look like we've fixed the memory bloat in the fldgen save files yet, so this adds up to a big chunk of memory.

I still think that having fldgen provide a function to extract the data we need is the right way to go, but at the very least, we need to manage the scope of these large objects very carefully.

Thinking on it a little more, I wonder if making this TgavStub a separate component is the right move at all. The FldgenComponent already reads in this data anyhow. What if we provided a configuration option to have FldgenComponent provide a tgav capability? Then, if that capability is turned on, fldgen would extract the needed information from the files it is already reading in, use those values of tgav itself, and export them to the rest of the framework. If not, then it skips that step and requests tgav from elsewhere in the framework.

A slight variant on that would be to implement the feature in issue #62. Then, configuring fldgen to compute its own tgav would always trigger the export, but a user could squelch that if they wanted to have the rest of the system get tgav from somewhere else. That would provide the most flexibility.

Same comment as above; this function belongs in a utility module.

This is another function that belongs in a utility module.

I think we can get away without this function, if we create an exported function in fldgen to extract the information we need and return it as a simple vector.

I'm not sure I understand what is going on here. Are you extracting the names of netCDF files from the RDS file? I don't think it's necessary to go back to the original data to extract this information. I'm pretty sure we store the global mean temperature series in the fldgen save file. If not, then it would be easy to do so, since we have to calculate it as part of the training process. We definitely don't want to be going back to the original data. First, it's brittle (What if the original data files move? Or what if you want to take the calculation to a new computing system?), and second it's unnecessarily slow.

See my comments above about a better way to handle metadata. I really think that insofar as possible, we need to generate this information down in the framework's classes. We could potentially create a hook for a component to add extra metadata beyond what the framework can glean from the input configuration, but it should be optional.