Skip to content

Conversation

colemanjs
Copy link
Collaborator

Projects gaussian heat source which uses a supergaussian axial decay function, where the coefficient on the decay function can be specified as a function of the heat source aspect ratio (a), such that: n = A*log2(a) + B

@colemanjs colemanjs self-assigned this Oct 16, 2025
Copy link
Collaborator

@gknapp1 gknapp1 left a comment

Choose a reason for hiding this comment

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

Quick review while still in draft state. Looks good from reading through the code, but request a re-review once this exist draft state and I'll test it.

If you have any references for the origin of the mathematical form, would be good to include them in the source code somewhere for documentation.

Comment on lines +124 to +128
heatSourceModelCoeffs_ = optionalSubDict(type() + "Coeffs");

//- Mandatory entries
heatSourceModelCoeffs_.lookup("A") >> A_;
heatSourceModelCoeffs_.lookup("B") >> B_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't optionalSubDict contradictory with the mandatory entries comment?

Copy link
Collaborator Author

@colemanjs colemanjs Oct 17, 2025

Choose a reason for hiding this comment

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

@kincaidkc - this is carried over from the original refactor of the heatSourceClass to handle polymorphism. Do you remember why we are using optionalSubDict here? I know that there are several examples in OpenFOAM-10 using this method for similar uses, (example) but haven't tested changing this yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It lets you put the coefficients inside or outside of the subdictionary. For model-specific stuff we should probably require a subdictionary.

Copy link
Collaborator Author

@colemanjs colemanjs Oct 17, 2025

Choose a reason for hiding this comment

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

@kincaidkc, so optionalSubDict should be moved to subDict if we want to make sure that mandatory entries are read from type()Coeffs? I can test this out, but we will probably was to create a PR to do this for all derived heatSourceModels and absorptionModels.

@colemanjs colemanjs marked this pull request as ready for review October 16, 2025 20:30
@colemanjs
Copy link
Collaborator Author

Quick review while still in draft state. Looks good from reading through the code, but request a re-review once this exist draft state and I'll test it.

If you have any references for the origin of the mathematical form, would be good to include them in the source code somewhere for documentation.

We don't have a citation for the mathematical form yet. It is the focus of a manuscript I am currently preparing.

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.

3 participants