clawpack / visclaw

Clawpack visualization tools
http://www.clawpack.org
BSD 3-Clause "New" or "Revised" License
29 stars 47 forks source link

Auto-detect output format #220

Closed rjleveque closed 7 years ago

rjleveque commented 7 years ago

Currently plotdata.format in setplot.py has to be consistent with clawdata.output_format in setrun.py. If one is ascii and the other binary then obscure errors happen without explanation.

In the proposed change to data.getframe, if format is set to None, it checks if there are any fort.b files in the outdir and if so assumes binary, otherwise ascii.

Are hdf5 or netcdf in use and should we try to auto-detect those?

Note: This change is needed to allow a single setplot to load data from two different outdir's (for comparison plots) in the case they have different formats, not currently possible. A future PR will support this.

ketch commented 7 years ago

This looks good. Other formats that are in use include hdf5 and PyClaw's "petsc" output format. It would be great if those were added to the auto-detect.

mandli commented 7 years ago

I am about to endeavor on an effort to really implement NetCDF output but these too could be indentified with the extra files and suffix.

rjleveque commented 7 years ago

Oops, this breaks if plotdata.format = None at the line

        self.output_controller = clawpack.pyclaw.controller.OutputController(
                                           self.outdir, file_format=self.format)

in data.py. I didn't test it as well as I thought.

@ketch, @mandli: Do we really need to set the format when self.output_controller is initialized? For the use case I have in mind the same controller might be used to read from different directories that are in different formats.

Also, what file extensions should we look for to auto-detect hdf5 and petsc?

mandli commented 7 years ago

The output_controller as implemented to keep the file_format persistent as sometimes it would get reset by defaults in other places. The suffix for HDF5 is "hdf". For PETSc I think it is "ptc" but I am not certain on that one as petclaw and for that matter forestclaw can write out in ascii and binary.

ketch commented 7 years ago

Yes, .ptc is the extension for the "petsc" format. Really it's just a binary file with a particular custom format.

I honestly don't remember at all how the OutputController works or what might break by setting its file_format to None.

mandli commented 7 years ago

The output_controller is really a hack, it would be great if we could figure out a better way to handle format. Maybe it is better to have the Solution class or the fileio package attempt to auto-detect the file format. The advantage of fileio doing it would be that it would be forestclaw, petclaw, and pyclaw specific and would keep those questions out of VisClaw which I think is a good idea.

ketch commented 7 years ago

keep those questions out of VisClaw which I think is a good idea

Yes, I would rather that VisClaw didn't read files at all. It ought to really just deal with Solutions in memory. A few years ago I made some changes in this direction, but it's too difficult to separate it all out at this point.

Why not put the functionality proposed in this PR into the pyclaw.Solution object?

donnaaboise commented 7 years ago

As a VisClaw user, I'd love to see just the opposite - a standalone VisClaw that can work without any other Clawpack repositories. I realize this would be a major reorganization, (and may not even fit into the current clawpack/pyclaw paradigm) but VisClaw could be really useful on its own. I'd vote for a flexible way for users to plug-in their own readers, which of course could be supplied by Clawpack for clawpack users, or supplied as stand-alone readers for people using other packages.

ketch commented 7 years ago

@donnaaboise I don't think we disagree here. I would like to see the plotting code know nothing about loading data files; i.e., have a clean well-defined interface between the two. Whether the code that loads files lives in the same repository as the vis code is a separate question. In the Griddle project I started (but didn't finish), loading of solutions was to be incorporated into the same package as visualization, while still carefully keeping the two orthogonal.

Basically, I think we agree that some parts of PyClaw and VisClaw would work really great if separated out into their own package. That package wouldn't need to know anything about the rest of Clawpack and might be useful to a lot of people who don't use Clawpack.

donnaaboise commented 7 years ago

@ketch Actually, I am advocating for a vis package that does know how to read files. Or that has a generic interface to either clawpack supplied or user supplied readers. I do agree that where they are stored is probably a separate issue.

ketch commented 7 years ago

As I said, my plan was to have the package include loading of files. But the functions/subroutines that plot things would not know or care about things like file formats and directories. I don't see a good reason to entangle those the way they are, for instance, in frametools.

Indeed, if I get back to developing griddle, the next step was to move the fileio and Solution stuff out of PyClaw and into Griddle. I think that's exactly what you're asking for.

Anyway, I think we're both just repeating the same things.

rjleveque commented 7 years ago

I agree this needs more discussion before implementing, so I'm closing this broken PR and opened #221 so we don't forget about it.

ketch commented 7 years ago

I'm sorry; I didn't mean for our discussion to completely derail this. A quick solution might be worthwhile, even if we are planning something more involved later.

mandli commented 7 years ago

I have something in a branch for PyClaw. Let me make a PR so we can at least continue to have a discussion. I actually think it may still be prudent to set plotdata.format to None unless explicitly overridden.

mandli commented 7 years ago

Check to see if clawpack/pyclaw#581 addresses this.