Added functionality: reading from an external source file#194
Conversation
* Added class externalSource extending source_inter. Source file can be read in binary or in ASCII, using the formatting of printSource. * Added functionality to printStatement to use binary format instead of ASCII. Added a new readBinary switch to physicPackages.
|
A small, quick comment: I would change the name to, perhaps, 'fileSource'. This is because 'external source' is commonly used interchangeably with 'fixed source', so it might be a bit ambiguous. |
valeriaRaffuzzi
left a comment
There was a problem hiding this comment.
Thanks a lot for the development, this is very nice!! I just have a few comments and questions. Aside from those I left throughout the code, more high level things:
- I do agree with Paul about changing the name to fileSource
- We should also update the docs, i.e., the InputManual to include the change.
| if (writeBinary) then | ||
| filename = trim(name)//'.bin' | ||
| open(unit = id, file = filename, status = 'replace', access = 'stream', form = 'unformatted') | ||
|
|
||
| ! Print out each particle co-ordinate | ||
| do i = 1, self % pop | ||
| write(id) self % prisoners(i) % r, self % prisoners(i) % dir, & | ||
| self % prisoners(i) % E, real(self % prisoners(i) % G, defReal), & | ||
| real(self % prisoners(i) % broodID, defReal) | ||
| end do | ||
| else | ||
| ! Print out each particle co-ordinate | ||
| filename = trim(name)//'.txt' | ||
| open(unit = id, file = filename, status = 'replace') | ||
|
|
||
| ! Print out each particle co-ordinate | ||
| do i = 1, self % pop | ||
| write(10, *) self % prisoners(i) % r, self % prisoners(i) % dir, & | ||
| self % prisoners(i) % E, self % prisoners(i) % G, & | ||
| self % prisoners(i) % broodID | ||
| end do | ||
| ! Print out each particle co-ordinate | ||
| do i = 1, self % pop | ||
| write(id,*) self % prisoners(i) % r, self % prisoners(i) % dir, & | ||
| self % prisoners(i) % E, self % prisoners(i) % G, & | ||
| self % prisoners(i) % broodID | ||
| end do | ||
| end if |
There was a problem hiding this comment.
Could you have something like this, where the loop over particles is only written once?
- Why do you need real() to write to binary and not for ascii?
- I think the group should be an integer, since it is an index.
| if (writeBinary) then | |
| filename = trim(name)//'.bin' | |
| open(unit = id, file = filename, status = 'replace', access = 'stream', form = 'unformatted') | |
| ! Print out each particle co-ordinate | |
| do i = 1, self % pop | |
| write(id) self % prisoners(i) % r, self % prisoners(i) % dir, & | |
| self % prisoners(i) % E, real(self % prisoners(i) % G, defReal), & | |
| real(self % prisoners(i) % broodID, defReal) | |
| end do | |
| else | |
| ! Print out each particle co-ordinate | |
| filename = trim(name)//'.txt' | |
| open(unit = id, file = filename, status = 'replace') | |
| ! Print out each particle co-ordinate | |
| do i = 1, self % pop | |
| write(10, *) self % prisoners(i) % r, self % prisoners(i) % dir, & | |
| self % prisoners(i) % E, self % prisoners(i) % G, & | |
| self % prisoners(i) % broodID | |
| end do | |
| ! Print out each particle co-ordinate | |
| do i = 1, self % pop | |
| write(id,*) self % prisoners(i) % r, self % prisoners(i) % dir, & | |
| self % prisoners(i) % E, self % prisoners(i) % G, & | |
| self % prisoners(i) % broodID | |
| end do | |
| end if | |
| if (writeBinary) then | |
| filename = trim(name)//'.bin' | |
| open(unit = id, file = filename, status = 'replace', access = 'stream', form = 'unformatted') | |
| else | |
| ! Print out each particle co-ordinate | |
| filename = trim(name)//'.txt' | |
| open(unit = id, file = filename, status = 'replace') | |
| end if | |
| ! Print out each particle co-ordinate | |
| do i = 1, self % pop | |
| write(id) self % prisoners(i) % r, self % prisoners(i) % dir, & | |
| self % prisoners(i) % E, real(self % prisoners(i) % G, defReal), & | |
| real(self % prisoners(i) % broodID, defReal) | |
| end do |
| ! Update RNG after it was used to normalise particle population | ||
| call self % pRNG % stride(1) | ||
|
|
||
| ! Print source in ASCII format if requested |
There was a problem hiding this comment.
Could also avoid the repeated call by setting a binary flag that's inputted into the function printToFile
| ! Update RNG after source generation | ||
| call self % pRNG % stride(self % totalPop) | ||
|
|
||
| ! Print source in ASCII if requested |
There was a problem hiding this comment.
Same comment as for the eigenPP
SharedModules/genericProcedures.f90
Outdated
| output = temp | ||
| case (iostat_end) | ||
| EOF = .true. | ||
| return |
There was a problem hiding this comment.
Surely you don't need the return, since there is nothing after the select case.
SharedModules/genericProcedures.f90
Outdated
| end function printParticleType | ||
|
|
||
| !! | ||
| !! Read a line from the source file in ASCII format |
There was a problem hiding this comment.
The comment shoudl be 'ASCII or binary' I believe?
There was a problem hiding this comment.
I would write a comment about what EOF is.
| allocate(self % r(3, self % numberNeutrons)) | ||
| allocate(self % dir(3, self % numberNeutrons)) |
There was a problem hiding this comment.
| allocate(self % r(3, self % numberNeutrons)) | |
| allocate(self % dir(3, self % numberNeutrons)) | |
| allocate(self % r(3, self % numberNeutrons), self % dir(3, self % numberNeutrons)) |
| ! Read and store neutron data from source file | ||
| ! Value for BroodID is ignored | ||
| print *, self % numberNeutrons, ' neutrons read from external source file' | ||
| do i = 1, self % numberNeutrons |
There was a problem hiding this comment.
Is there a way to avoid looping through the file twice? Does this take long?
There was a problem hiding this comment.
I wouldn't think so, to answer both questions. A scan is probably necessary for the allocation.
| ! Get pointer to neutron material, return fatal error if not found | ||
| mat => neutronMaterial_CptrCast(nucData % getMaterial(matIdx)) | ||
| if (.not.associated(mat)) call fatalError(Here, "Nuclear data did not return neutron material.") |
There was a problem hiding this comment.
The material doesn't seem to be used, don't think we need this.
| p % dir = self % dir(:, idx) | ||
| p % type = P_NEUTRON | ||
| p % time = ZERO | ||
| p % wgt = ONE |
There was a problem hiding this comment.
It would be nice to read the weigth of the particle too!
| call dungeon % replace(self % sampleParticle(pRand), i) | ||
| end do | ||
| !$omp end parallel do | ||
| self % internalCounter = 1 |
There was a problem hiding this comment.
Not sure I understand what this is for, since it never gets updated or incremented?
* Some style changes and code cleaning
| character(100), parameter :: here = 'readASCII_defReal (genericProcedures.f90)' | ||
| integer(shortInt), intent(in) :: unit | ||
| logical(defBool), intent(in) :: readBinary | ||
| integer(shortInt), intent(out), dimension(:) :: output |
There was a problem hiding this comment.
Sorry to be pedantic, we tend to do
integer(shortInt), dimension(:), intent(out) :: output
| id = 10 | ||
| ! Open the file in requested mode | ||
| if (writeBinary) then | ||
| if (writeBinary == 2) then |
There was a problem hiding this comment.
I like this solution! I think it might be more readable to include a parameter either to this file or to universalVariables like
BINARY_FILE = 2
and then here you could have
if (fileType == BINARY_FILE) then .....
* Added a parameter variable in particleDungeon_class.f90 for writting binary file * Modified CMakeList.txt to hopefully fix the check failure
| !! - failed to read source file | ||
| !! | ||
| subroutine init(self, dict, geom) | ||
| class(fileSource), intent(inout) :: self |
There was a problem hiding this comment.
Align with the other variables
|
|
||
| type, public, extends(source) :: fileSource | ||
| private | ||
| integer(shortInt) :: numberNeutrons |
There was a problem hiding this comment.
Probably better as a longInt if we want quite big sources.
| ! Read source data from file, binary or ASCII | ||
| self % numberNeutrons = 0 | ||
| id = 10 | ||
| ! ASCII file reading |
There was a problem hiding this comment.
I would add a few more spaces for readability I think
| else | ||
| nucData => ndReg_getNeutronCE() | ||
| end if | ||
| if(.not.associated(nucData)) call fatalError(Here, 'Failed to retrieve Nuclear Database') |
There was a problem hiding this comment.
It isn't clear to me why knowledge of the nuclear database is necessary for this source. I think it can be removed entirely.
|
|
||
| id = 10 | ||
| ! Open the file in requested mode | ||
| if (writeBinary == BINARY_FILE) then |
There was a problem hiding this comment.
It isn't clear to me why this can't just be a bool, which would remove the need for the parameter defined above.
| if (self % printSource /= 0) then | ||
| call self % nextCycle % printToFile(trim(self % outputFile) // '_source' // numToChar(i) // & | ||
| '_rank' // numToChar(getMPIRank())) | ||
| '_rank' // numToChar(getMPIRank()), self % printSource) |
There was a problem hiding this comment.
Ah, I guess this answers my last question. Maybe replace this with self % printSource == BINARY_FILE, where BINARY_FILE is defined in universalVariables. I think there are a few other places we might want to use that variable in future.
| ! Read whether to print particle source per cycle | ||
| ! Read whether to print particle source per cycle, 1 for ASCII, 2 for binary | ||
| call dict % getOrDefault(self % printSource, 'printSource', 0) | ||
| if (self % printSource < 0 .or. self % printSource > 2) then |
There was a problem hiding this comment.
This would likewise use the above mentioned variable.
| call self % thisCycle % printToFile(trim(self % outputFile)//'_source'//numToChar(i)) | ||
| ! Print source in ASCII or binary format if requested | ||
| if (self % printSource /= 0) then | ||
| call self % thisCycle % printToFile(trim(self % outputFile)//'_source'//numToChar(i), self % printSource) |
* Various style fixes * Removed retrieval of dataBase from fileSource
valeriaRaffuzzi
left a comment
There was a problem hiding this comment.
This looks good to me, and if @ChasingNeutrons is also happy with the changes he requested we can merge!
Thanks a lot @bonnettheophile for the development 💯
| use neutronMaterial_inter, only : neutronMaterial, neutronMaterial_CptrCast | ||
| use nuclearDataReg_mod, only : ndReg_getNeutronCE => getNeutronCE, & | ||
| ndReg_getNeutronMG => getNeutronMG | ||
| use nuclearDatabase_inter, only : nuclearDatabase |
There was a problem hiding this comment.
You can also remove references to nuclear data here.
| character(100),parameter :: Here = 'sampleParticle (fileSource_class.f90)' | ||
|
|
||
| ! Sample index of source neutron to be used | ||
| idx = int(rand % get() * real(self % numberNeutrons, defReal)) + 1 |
There was a problem hiding this comment.
idx needs to be a longInt if there is a large number of particles.
Added class externalSource extending source_inter. Source file can be read in binary or in ASCII, using the formatting of printSource.
Added functionality to printStatement to use binary format instead of ASCII.
In physicsPackages, setting printSource 2; prints to binary format
Modified files:
eigenPhysicsPackage_class.f90
fixedSourcePhysicsPackage_class.f90
sourceFactory_func.f90
source_inter.f90
particleDungeon_class.f90
genericProcedures.f90
Added file: externalSource_class.f90