Write table on fortran file type.

[djabaay]

Adding functionality to write a 2-D string array as a table on the fortran file type.

[djabaay]

@sooyoung0321 When you've got some free time, please have a look at the code for a review.

[stimpsonsg]

What ticket is this for?

[djabaay]

https://vminfo.casl.gov/trac/casl_phi_kanban/ticket/6089

[aarograh]

Just out of curiosity, why is this a type-bound procedure on the file type? None of the formatting work requires the file object at all. Seems like the formatting of the 2D strings into a table should be a separate function from the write function.

[stimpsonsg]

ok, I'm seeing no mention of the ticket number in any of the commits or anything...it needs to be there

[djabaay]

@stimpsonsg Agreed, missed that. I'll go back and add. @aarograh Is it worth it to convert the 2-D string array and process it into a 1-D string array that can just be looped over and written to a file? Is that essentially what you're asking?

[aarograh]

@aarograh Is it worth it to convert the 2-D string array and process it into a 1-D string array that can just be looped over and written to a file? Is that essentially what you're asking?

Yeah, that's essentially what I had in mind. The formatting of the 2D array of strings seems to me to be a separate action from writing it to a file. And it's conceivable that one would want to use that capability to write to stdout or using WRITE(unit_number,*) or something, even if just for debugging purposes.

If there's a compelling reason to leave it this way, that's totally fine. But it seems like much of the writeTable_fortran_file method could just be cut out into a separate function that's called from writeTable_fortran_file.

[djabaay]

@aarograh Is it worth it to convert the 2-D string array and process it into a 1-D string array that can just be looped over and written to a file? Is that essentially what you're asking?

Yeah, that's essentially what I had in mind. The formatting of the 2D array of strings seems to me to be a separate action from writing it to a file. And it's conceivable that one would want to use that capability to write to stdout or using WRITE(unit_number,*) or something, even if just for debugging purposes.

If there's a compelling reason to leave it this way, that's totally fine. But it seems like much of the writeTable_fortran_file method could just be cut out into a separate function that's called from writeTable_fortran_file.

So, how about something like stringTableToLines() that lives in IO_Strings.f90 taking a 2-D formatted string array that changes it into a 1-D array of strings that can be looped over and written to a file. The question is, where should the write linesToFile() or whatever live? Still here? Should that also be in IO_Strings.f90?

Or do we want to fold stringTableToLines into https://github.com/CASL/Futility/pull/203, since I don't envision creating a table like this without doing the ParameterList step first?

[aarograh]

I definitely agree with the stringTableToLines() in IO_Strings.f90. I think having a method on the file type still makes sense to actually write the 1D array of strings to a file. I'm kind of thinking how the HDF5 file works with a GENERIC :: write, but only implementing the 1D StringType version right now for the fortran file type.

[djabaay]

Roger that, I can break stuff up and move it around.

[djabaay]

Rebased master onto the branch, but forgot to update the commit messages. The writeTable routine is now split up as @aarograh requested, and the commit messages have PHI ticket numbers as @stimpsonsg requested.

[djabaay]

Ran the Nightlies with MPACT:

99% tests passed, 6 tests failed out of 750

Subproject Time Summary:
CTeuchos         =   1.32 sec*proc (4 tests)
ForTeuchos       =   2.86 sec*proc (4 tests)
Futility         =  44.19 sec*proc (60 tests)
MPACT_API        = 240.85 sec*proc (4 tests)
MPACT_Drivers    =   2.64 sec*proc (4 tests)
MPACT_exe        = 126447.21 sec*proc (498 tests)
MPACT_libs       = 1088.74 sec*proc (176 tests)

Label Time Summary:
BASIC         =  44.19 sec*proc (60 tests)
CONTINUOUS    =  44.19 sec*proc (60 tests)
HEAVY         =  44.19 sec*proc (60 tests)
NIGHTLY       =  44.19 sec*proc (60 tests)

Total Test time (real) = 8510.05 sec

The following tests FAILED:
    321 - MPACT_exe_testValid_verify_subplane_rod_coupled (Failed)
    391 - MPACT_exe_testValid_transient_4-mini_3D (Failed)
    392 - MPACT_exe_testValid_transient_4-mini_1GAccel (Failed)
    393 - MPACT_exe_testValid_transient_4-mini_3D_SCRAM (Failed)
    396 - MPACT_exe_testValid_steadystate_4mini_crw (Failed)
    397 - MPACT_exe_testValid_transient_4mini_crw (Failed)
Errors while running 

subplane_rod_coupled fails because I didn't compile with CTF (I have a commit to IFDEF this in CMake with ENABLED_CTF). The others fail because of the negative fluxes with DBC.

DBC Failure: .NOT. ANY(tmpTS%phis(:,ig) < 0.0_SRK) in /home/djabaay/MPACT/MPACT_libs/CoreSolvers/src/FixedSrcSolver.f90 on line  953:  process   14 of   16

[stimpsonsg]

I think we need to reconsider this DBC check. The scalar flux can go negative without being completely catastrophic.

[djabaay]

That's fine, I can make a MR on our end and remove them if that is the consensus.

[bscollin]

I do think we need a check on the final solution to ensure its positive and throw a warning, but agree during the iteration, things going negative isn't the end of the world. It might run into issues if negative fluxes get passed to ORIGEN (but that would need to be a negative total flux... group integrated).

[djabaay]

That makes sense. Would ORIGEN need to ensure that all source region fluxes for all groups on a single cross section region sum to a positive value at the end of a solve?

[djabaay]

@aarograh Pushed updates based on your feedback. I added the ticket number to the last few commits, which should show up in the merge commit when squashed, right?

[djabaay]

@aarograh, I pushed changes, when you can, let me know if those comments are resolved.

[aarograh]

Looks ok to me. @stimpsonsg or @bkochuna can merge whenever

[bkochuna]

Merging since @aarograh has given the greenlight.