FireDynamics / fdsreader

Python reader for FDS data
GNU General Public License v3.0
44 stars 18 forks source link

Limited functionality for simulations ran with nightly build FDS version #72

Open ManuelOsburg opened 5 months ago

ManuelOsburg commented 5 months ago

I have identical two FDS simulations. One was run with FDS-6.8.0-release and another with FDS-6.8.0-1386-g42fde33-nightly. While there are no problems with the simulation calculated with FDS-release, the fdsreader functionality is limited with the simulation calculated with FDS-nightly. In particular, the BNDF files cannot be read.

FDS-release image

FDS-nightly: image

JanVogelsang commented 5 months ago

Yes, that might very well be the case, as the nightly build incorporates wild changes of the FDS development team that might very well never make it to an actual release. As the FDS team is not supporting this project in any way, it's impossible to keep up with their daily changes. It's even difficult to make the FDSreader compatible with various FDS release versions, as the FDS team is not managing to make their versions backward-compatible.
So unfortunately, we can't offer any support for nightly builds. They might work, but might also not work. You can create an issue at FDS thought and indicate that their nightly builds are not producing the same (binary) outputs as their release versions, but I would assume they are aware of that and simply don't guarantee any consistency between versions.

amarcozzi commented 5 months ago

Hi, @JanVogelsang. Now that these changes have transitioned from the nightly release to the new minor release version 6.9 of FDS, would you be open to integrating this change? I have a member of my team that can work on a fix for the new output file format.

JanVogelsang commented 5 months ago

@amarcozzi Sure thing, such a fix would be very welcome! He could either create a pull request directly or just describe how the file format has changed, so that I can integrate the changes into the code!

amarcozzi commented 5 months ago

Thanks, @JanVogelsang. My teammate and I talked about this yesterday and she's going to start looking into a fix. This is related to issue #73 as well just so you're aware.

briannak7 commented 5 months ago

@JanVogelsang, the FDS developers informed me that the .bnd files have been changed and no longer output the minimum and maximum values for each timestep.

When looking at the source code for fdsreader, it appears that Simulation._load_boundary_data is reading the number of timesteps (n_t) from these .bnd files. Consequently, when the Patch object is created in this method, n_t is set to 1 due to the file changes. This is why, I think, we’re only seeing the one timestep of boundary data.

Since these files are now differently structured, I wondered if reading in the data from the .bf files rather than the .bnd files would be a good approach. If so, any ideas on how to start with this change would be appreciated!

JanVogelsang commented 5 months ago

@briannak7 Are they now outputting the number of time steps somewhere in .bf files instead? If not it's quite difficult to read in the data, as you can't just read in binary data without knowing how you should actually interpret the data. It might be possible to calculate the number of time steps using the file size though, I think I have done that for some other data type as well.

briannak7 commented 4 months ago

@JanVogelsang, the .bf files don't contain the number of time steps directly, but we can grab the lower and upper bounds at each timestep within the .bf file and set n_t = len(times). It looks like we're already getting data from this file in _load_boundary_data and this could be a simple addition.

I wasn't sure if the boundary data is lazy loaded or not. If not, reading in the data this way could be time consuming and getting the number of timesteps from the file size may be optimal upon initializing the simulation.

I have working code to read in the .bf file, and I'm currently working on incorporating it into Simulation._load_boundary_data. I hope to have this finished later this week. Let me know if you have any thoughts or feedback!

JanVogelsang commented 4 months ago

@briannak7 So the times or lower/upper bounds are now stored in the .bf-file? Or how would we get "len(times)"?

Yes, the patch data (boundary data for one side of an obstruction of one mesh) is lazy loaded, but the .bf-file is still read to load some meta-data, but only the file header is actually read. Looking forward to seeing your implementation!

briannak7 commented 4 months ago

@JanVogelsang the times, lower, and upper bounds are stored in the .bf file which is how we can get len(times).

Thanks for letting me know this is lazy loaded!

JanVogelsang commented 4 months ago

But how do you load the times without knowing how many times to load? Sounds like a chicken and egg problem.

amarcozzi commented 2 months ago

Hi @JanVogelsang sorry for the delay on this. I implemented a quick fix and created PR #79 so that you could see the change. I intend to update PR #79 with feedback from this issue discussion.

To refresh our memory about the problem, the .bnd files containing the time, lower, and upper bounds and timestep are deprecated as of FDS 6.9. Rather than reading that data from the .bnd text files, I propose that we read the data directly from the binary files.

PR #79 implements this for the boundary data by reading the headers, patch information, and computing n_t based on the number of bytes in the file. Then, it reads in the time for each timestep by updating the file offset. This approach no longer reads in the lower and upper bounds data because it is quite computationally expensive to allocate an array for each patch, compute the min/max, then do that for each timestep. It would also in effect read in the data twice. So I chose not to include those values.

Let me know what you think of this approach.