Skip to content

Conversation

@ChasingNeutrons
Copy link
Collaborator

Using the pieceConstantField, I have added the capability to super-impose temperature and density-scaling fields on the geometry. Both surface and delta tracking agree with each other for temperature and density changes, and they agree with reference calculations without imposed fields.

The distance calculations are done inside move. They work well for the most part, with one uncomfortable bit: fuzziness needed to be added to ensure that distance to the field does not take precedence over distance to the boundary. If there are any better suggestions for this it would be great.

This necessitated make the initMajorant function part of the nuclearDatabase, allowing it to be called in the physicsPackages which can receive the relevant field information from geometry.

I have updated the docs and added an integration test to geometry for obtaining values from the fields and distance calculations.

Added in the BEAVRS model with the D-bank partially inserted. Also fixed
a flaw in the geometry where the outermost cell was defined such that
there could be a rare particle lost between it and the geometry
boundary.
Field which have values that are piecewise constant. Endowed with a
distance calculation. Added a simple version which allows a lattice to
be specified.
Can overlay temperature and density fields on the geometry. Supports
both surface and delta tracking.
@ChasingNeutrons
Copy link
Collaborator Author

I have also added a new map to allow tallying on a given field definition. Currently this is limited to pieceConstantFields until we unify the interface.

Copy link
Member

@valeriaRaffuzzi valeriaRaffuzzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was a lot!!!! The nuclear data bit seemed to make sense but I have to admit it will be good to give it another look post revisions.

There are a couple of bigger issues that I wrote in specific comments, but in general it looks good! It was a massive effort so well done.

real(defReal) :: time ! Particle time point

! Information passed from geometry
real(defReal) :: T = -INF ! Local temperature
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use a consistent undefined value in between places: it is -INF here but, for example, -ONE in fissionSource_class.f90 (line 227)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I should define a 'NO_TEMPERATURE' and 'NO_DENSITY' I think.

in different materials, or uniformly across all materials.

- shape: (x y z) array of integers, stating the numbers of x, y and z
elements of the field. For a 2D field, one of the entries has to be 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought only z was allowed to be 0

- names of each material: a map, named after every material present in the materials list.
The entries of the map are the values that the field takes in that material in that
element of the field. The order is: increasing x, increasing y and then increasing z.
- default: the value taken by the field when a point is either outside of the field or
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should probably be a default for each material.
In the case of density, the default could be set to 1.0 for all materials but this wouldn't be possible for the temperature.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may need to think about a way of doing this in cases where there are many materials. The most obvious use case that I see at the moment is that there are ~2 materials one would care to modify (fuel, coolant). The others would not have any modifications, i.e., they would use the default input values, corresponding to setting a default of a negative number, which does not perform any modification. Maybe there is a question of how to make this as userfriendly as possible, e.g., the default default is 'do nothing'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think I understand better now. You mean for each material specified in the field definition. This does make sense; the default option was made for anything that doesn't correspond to what's in the map (either in space or material). There is maybe quite a multiplicity of options to consider...

Comment on lines +122 to +145
if (dict % isPresent('path')) then
call dict % get(path, 'path')
call self % init_file(path)
else
call self % init_dict(dict)
end if

end subroutine init

!!
!! Initialise field from a file
!!
!! Args:
!! file [in] -> Path to a file containing the field definition
!!
subroutine init_file(self, path)
class(pieceConstantField), intent(inout) :: self
character(pathLen), intent(in) :: path
type(dictionary) :: dict

call fileToDict(dict, path)
call self % init(dict)

end subroutine init_file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (dict % isPresent('path')) then
call dict % get(path, 'path')
call self % init_file(path)
else
call self % init_dict(dict)
end if
end subroutine init
!!
!! Initialise field from a file
!!
!! Args:
!! file [in] -> Path to a file containing the field definition
!!
subroutine init_file(self, path)
class(pieceConstantField), intent(inout) :: self
character(pathLen), intent(in) :: path
type(dictionary) :: dict
call fileToDict(dict, path)
call self % init(dict)
end subroutine init_file
if (dict % isPresent('path')) then
call dict % get(path, 'path')
call fileToDict(dict, path)
end if
call self % init_dict(dict)
end subroutine init

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then you remove init_file, you have init public and init_dict private

Comment on lines +106 to +107
!! T -> temperature of the cross section
!! rho -> density scaling of the cross section
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should T and rho have been added to cacheSingleXS?

! Procedures implemented by a specific CE Neutron Database
procedure(updateMajorantXS), deferred :: updateMajorantXS
procedure(updateTotalMatXS), deferred :: updateTrackMatXS
procedure(updateTrackMatXS), deferred :: updateTrackMatXS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could we have survived with this massive typo?



! This check is really awful - can we do something better?
else if (fieldDist < dist .and. abs(fieldDist - dist) > 10*NUDGE) then ! Stays within the same cell, but crosses field boundary
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you just do
else if (maxDist < dist .and. maxDist >= fieldDist)?
in this case you're not crossing any geometrical boundary but a field boundary.

!! Returns nucIdx <= if material is not fissile
!!
function sampleFission(self, E, rand) result(nucIdx)
function sampleFission(self, E, temp, rho, rand) result(nucIdx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all these sampling functions are not used to be honest...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants