Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-detect output format #220

Closed
wants to merge 3 commits into from

Conversation

rjleveque
Copy link
Member

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
Copy link
Member

ketch commented Jul 19, 2017

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
Copy link
Member

mandli commented Jul 19, 2017

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
Copy link
Member Author

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
Copy link
Member

mandli commented Jul 20, 2017

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
Copy link
Member

ketch commented Jul 20, 2017

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
Copy link
Member

mandli commented Jul 20, 2017

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
Copy link
Member

ketch commented Jul 20, 2017

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
Copy link
Contributor

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
Copy link
Member

ketch commented Jul 21, 2017

@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
Copy link
Contributor

@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
Copy link
Member

ketch commented Jul 21, 2017

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
Copy link
Member Author

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.

@rjleveque rjleveque closed this Jul 21, 2017
@ketch
Copy link
Member

ketch commented Jul 30, 2017

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
Copy link
Member

mandli commented Jul 30, 2017

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
Copy link
Member

mandli commented Jul 30, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants