Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding arbitrary (up to 50 of each) active and scalar fields #408

Merged
merged 32 commits into from
Nov 22, 2022

Conversation

cianwilson
Copy link
Member

No description provided.

feathern and others added 19 commits July 26, 2019 12:44
After resolving conflicts and attempting to move code appropriately this compiles but has not been tested.
…sing segfaults.

Note that the additional scalar field is currently mandatory.
Fix an error in bcs for svar and allow initial conditions to be specified (only through generic input).

Temporarily add a test for debugging purposes.
…tial Conditions.

Fixes segfault found using intel compiler.
…ons of all the parameters.

c(12) has now been taken for the buoyancy coefficient.
…ve or passive fields.

A limited number of output quantity codes are implemented currently.  For active fields these begin at 10000 and increment by 200
for each subsequent field.  For passive fields these begin at 20000 and also increment by 200.

main_input parameters are either indicated by chi_a or chi_p (active or passive).

A test is included to demonstrate the functionality on 2 active and passive fields, comparing to temperature, which does the exact
same thing.
@cianwilson cianwilson requested review from feathern and gassmoeller and removed request for gassmoeller September 16, 2022 22:26
@@ -12,6 +12,10 @@ class equation_coefficients:
f_dict = {'density':1, 'buoy':2, 'nu':3, 'temperature':4, 'kappa':5, 'heating':6,
'eta':7, 'd_ln_rho':8, 'd2_ln_rho':9, 'd_ln_T':10, 'd_ln_nu':11, 'd_ln_kappa':12,
'd_ln_eta':13, 'ds_dr':14}
c_chi_a_dict = {'diff_fact':1, 'source_fact':2, 'buoy_fact':3}
Copy link
Member Author

Choose a reason for hiding this comment

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

This can probably be ignored for now. It was my attempt at starting to deal with custom reference states but it is not complete and not necessarily the right way to do it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changes should be removed before merge (if not fixing custom reference states first)

tests/chi_scalar/run_test.sh Outdated Show resolved Hide resolved
@@ -62,11 +62,11 @@ Subroutine Initialize_Benchmark_Equations
Integer :: neq, nvar,lp, l, nlinks
Integer, Allocatable :: eq_links(:), var_links(:)
If (magnetism) Then
neq = 6
nvar = 6
neq = 6 + n_active_scalars + n_passive_scalars
Copy link
Member Author

Choose a reason for hiding this comment

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

These changes are redundant but may indicate to future developers that these fields need to be taken into account.

…in PDE coefficients rather than in the declaration of the

implicit terms.

This is intentionally done in only 3 out of the 4 definitions.  For custom reference state (not implemented yet) the user will have
to get the sign correct themselves.  This should ultimately be reflected in the documentation.
@cianwilson
Copy link
Member Author

Not ready to merge yet... still need to add room for extra ra_functions and ra_constants with custom reference states!

Copy link
Member Author

@cianwilson cianwilson left a comment

Choose a reason for hiding this comment

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

Looks like just a couple of files to delete.

@@ -0,0 +1,72 @@
&problemsize_namelist
Copy link
Member Author

Choose a reason for hiding this comment

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

Committed by mistake?

@@ -0,0 +1 @@
../c2001_case0_input_pscalar
Copy link
Member Author

Choose a reason for hiding this comment

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

Whole directory committed by mistake?

@@ -12,6 +12,10 @@ class equation_coefficients:
f_dict = {'density':1, 'buoy':2, 'nu':3, 'temperature':4, 'kappa':5, 'heating':6,
'eta':7, 'd_ln_rho':8, 'd2_ln_rho':9, 'd_ln_T':10, 'd_ln_nu':11, 'd_ln_kappa':12,
'd_ln_eta':13, 'ds_dr':14}
c_chi_a_dict = {'diff_fact':1, 'source_fact':2, 'buoy_fact':3}
Copy link
Member Author

Choose a reason for hiding this comment

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

Changes should be removed before merge (if not fixing custom reference states first)

@cianwilson cianwilson marked this pull request as ready for review November 22, 2022 16:03
Copy link
Contributor

@feathern feathern left a comment

Choose a reason for hiding this comment

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

Went through this with Cian. Going to merge.

n_variables = 6
n_equations = 6 + n_active_scalars + n_passive_scalars
n_variables = 6 + n_active_scalars + n_passive_scalars
if (n_active_scalars>0) then
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change the ">" to ".gt." -- for FORTRAN's sake.

@@ -207,6 +235,11 @@ Subroutine Load_Linear_Coefficients()
amp = -ref%Buoyancy_Coeff
Call add_implicit_term(peq, tvar, 0, amp,lp, static = .true.) ! Gravity --- Need LHS_Only Flag

do i = 1, n_active_scalars
amp = ref%chi_buoyancy_coeff(i,:)
Copy link
Contributor

Choose a reason for hiding this comment

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

The negative in the T term above is due to the decomposition into stream functions. Any negative signs associated with assumptions concerning light vs. heavy elements of Chi should be handled inside the buoyancy coefficient itself. So, here, need to remove negative sign and make appropriate adjustments in PDE_Coefficients.F90.

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