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

document and rename mass sources for transport #75

Closed
smolins opened this issue Feb 9, 2021 · 12 comments · Fixed by #91
Closed

document and rename mass sources for transport #75

smolins opened this issue Feb 9, 2021 · 12 comments · Fixed by #91
Assignees
Milestone

Comments

@smolins
Copy link
Contributor

smolins commented Feb 9, 2021

We need to document better the mass sources for transport and use more appropriate names for the Parameter Lists:

  1. There are currently two types of sources: "concentration" and "geochemical"

Examples of "concentration" from transport-column_infiltration.xml test

    <ParameterList name="concentration">
      <ParameterList name="Tc-99 injection">
        <Parameter name="component names" type="Array(string)" value="{Tc-99}" />
        <Parameter name="regions" type="Array(string)" value="{surface}" />
        <Parameter name="spatial distribution method" type="string" value="none" />
        <ParameterList name="source function">
          <ParameterList name="function-tabular">
            <Parameter name="x values" type="Array(double)" value="{0.0,8640000.0}" />
	<Parameter name="y values" type="Array(double)" value="{0.00111,0.0}" />
            <Parameter name="forms" type="Array(string)" value="{constant}" />
          </ParameterList>
        </ParameterList>
      </ParameterList>
      <ParameterList name="surface coupling">
        <Parameter name="regions" type="Array(string)" value="{surface domain}" />
        <Parameter name="spatial distribution method" type="string" value="domain coupling" />
        <Parameter name="submodel" type="string" value="rate" />
        <ParameterList name="fields">
          <Parameter name="flux_key" type="string" value="mass_flux" />
          <Parameter name="copy_flux_key" type="string" value="next_timestep" />
          <Parameter name="field_out_key" type="string" value="total_component_concentration" />
          <Parameter name="copy_field_out_key" type="string" value="default" />
          <Parameter name="field_in_key" type="string" value="surface-total_component_concentration" />
          <Parameter name="copy_field_in_key" type="string" value="default" />
        </ParameterList>
      </ParameterList>
    </ParameterList>

Example of "geochemical":

<ParameterList name="geochemical" type="ParameterList">
      <ParameterList name="Tracer injection" type="ParameterList">
        <Parameter name="regions" type="Array(string)" value="{surface}" />
        <Parameter name="solutes" type="Array(string)" value="{Tracer}" />
        <Parameter name="times" type="Array(double)" value="{0.0,8640000.0}" />
        <Parameter name="geochemical conditions" type="Array(string)" value="{west, west}" />
        <Parameter name="time functions" type="Array(string)" value="{constant}" />
    <Parameter name="ats units [moles/m^3]" type="bool" value="true" />
      </ParameterList>
    </ParameterList>
  1. "Concentration" with spatial distribution=none is basically a fixed mass rate, independent of any water sources. So if one does not add water but adds mass of a component, the concentration will increase. The name "concentration" is clearly a misnomer.

  2. "Concentration" with spatial distribution="domain coupling" is a to couple fluxes between surface and subsurface compartments.

  3. "Geochemical" requires a water source and calculates a mass rate by dividing the concentration values from a geochemical condition (obtained via Alquimia) by the water source rate. If the water source rate is zero, then no mass is added. In other words, the concentration in the surface water can only be as high as what is set in the input source (in the absence of reactions, naturally).

Based on these observations above:

  1. It appears as if the "concentration" mass source should be named "flux" or "rate"
  2. It appears as if there should be a new "concentration" mass source that could include the "geochemical" mass source based on Alquimia, and potentially in the future a new mass source where the concentration value is given natively in Amanzi-ATS, for regular conservative transport
@smolins smolins changed the title potential mass balance error in solute transport when using "concentration" sources and a surface-mass_source? document and rename mass sources for transport Feb 19, 2021
@smolins
Copy link
Contributor Author

smolins commented Feb 22, 2021

It would usually be preferable to incorporate any changes to the spec as soon as possible so that we create the least confusion in the future. So the question is whether we want any of this part of the next release (v1.1?).

@ecoon
Copy link
Collaborator

ecoon commented Feb 22, 2021

Yes, I would like to have this settled before release. Those names have given us a ton of trouble trying to reverse engineer what they do.

@ecoon
Copy link
Collaborator

ecoon commented Feb 24, 2021

@saubhagya-gatech @ajkhattak Can you guys touch base with Sergi and agree on some names either by this ticket or by a quick meeting? Would be good to get these agreed upon and documented.

@ajkhattak
Copy link
Contributor

To my understanding, in its current form, we have (as Sergi mentioned)

  1. concentration source has units: moles of solute/(m^2 * second)
  2. water mass has units: moles of water / (m^2 * second)

How about calling them:

  1. solute_source_rate (for standalone ats)
  2. water_source_rate (for geochemical)

If we assume, the name "source" by default includes "rate" then we can remove the "_rate".

Another option would be to include explicit "water source" in the input file for ATS-only transport (no Alquimia) too, i.e., independent of transport (reactive or non-reactive) user should provide "water source". In that case, for conservative transport (ats-only) the user will provide both solute_source and water_source, and for reactive transport water source will be provided in the input file and solute_source will be picked through Alquimia. Just a thought.

@smolins
Copy link
Contributor Author

smolins commented Feb 24, 2021

I am not sure if I follow everything you say Ahmad. The water_source_rate for the "geochemical" source is typically given in the State and given as m/s if in the surface (for precipitation). In any case, I think I would go for something simpler

1 . mass_rate (for setting a rate that adds mass of a transported quantity) with units of mol/m2/s.
2. geochemical (for setting the concentrations via an Alquimia constraint). Units handled by the interface. Tied to water source in State (that is, no water added means no solute added)
3. concentration (for setting a concentration with ATS). Units of mol-solute/mol-H2O. Tied to water source in State (that is, no water added means no solute added). Currently not coded in the ats transport pk but I think it would used more than option 1.
4. domain coupling (for coupling surface-subsurface)

To me, other than the question about naming there is the question about hierarchy. I am not sure why the option 1 and option 4 are currently under the same type of source (currently (mis)named "concentration"). Unless there is a strong argument against it, I would also suggest to flatten this hierarchy.

@ecoon
Copy link
Collaborator

ecoon commented Feb 24, 2021

  1. I would prefer component_mass_source or something similar, agree on units. This is in comparison to what I'm starting to think should be the default name in ATS, "water_mass_source" or "water_volumetric_source" to distinguish between [mol m^-2 s^-1] vs [m s^-1].

  2. right

  3. I agree that it would be nice to have this in ATS. And agree that it should be called something like "source_concentration" or "component_source_concentration" by default.

The only trickiness in 1&4 being in the same concept is that there are several more of these, for instance well models (were the source is in mol/s, and the source is evenly divided across volume of the well's region), and a few others. I'm good with flattening them.

Flattening the changes may require either generating xml or changes to Amanzi, as beyond the "top level" name of "concentration" or "geochemical" I think all of that gets parsed in Amanzi's TransportDomainFunction code. I'm not against that, but we should check with Konstantin before changing DomainFunctions because they get used by a lot of Amanzi's PKs.

@smolins
Copy link
Contributor Author

smolins commented Feb 24, 2021

  1. I like component_mass_source. I was stuck with "solute" which I did not like. Consistency with other named mass sources is important.
  2. component_source_concentration sounds good to me, or maybe even component_concentration_source could work.
  3. not to complicate things more but maybe then geochemical_source would be a better choice.

It sounds to me that flattening would be the preferable option but it is unlikely we can get the needed changes in before the next release, so I would focus on picking the names and leave potential additional development for later. At worst, we would be creating a separate source for "domain coupling" at a later date.

@ajkhattak
Copy link
Contributor

  1. I like component_mass_source [moles/(m2*s)] too.

  2. Currently we use the name "mass_source" [moles_water/(m2 * s)] in the state for water source when using geochemical condition. Are you guys saying the names will changes like:
    geochemical --> component_concentration_source
    mass_source --> water_mass_source (??)

Just trying to understand.

@smolins
Copy link
Contributor Author

smolins commented Feb 24, 2021 via email

@ecoon
Copy link
Collaborator

ecoon commented Feb 24, 2021

No matter how it is done, the exact name should be left to the user. Keys::readKey() provides a code-wide paradigm for reading keys from the input file, and we should allow the user to call the water mass source whatever they want.

Sergi, realize also that mass_source is by default mol/m^s/s, and you are exploiting a magic feature of surface flow when you use m/s. :D I don't want to depend upon that feature in transport, because it isn't even valid for subsurface transport, and by default the surface mass source is mol/m^2/s (you have to tell surface flow "mass source in meters"=True to get m/s). So it will be assumed that the transport code is provided with a water mass source in mol/m^2/s (meaning @saubhagya-gatech bug #72 needs to be fixed too).

@ecoon
Copy link
Collaborator

ecoon commented Feb 24, 2021

As for component_concentration_source, it would be a new one to give concentrations of components, tied to water sources.

We should check with @lipnikov on this -- does Amanzi currently have a way of providing a mass of water source and a concentration of the incoming source to the transport PK? Obviously one could externally multiply these two things and provide a mass of component source, but doing the multiply in the code is handy from a user perspective, especially for flow + transport runs where you need to supply the mass of water source to the flow PK already. Does such a TransportDomainFunction exist in Amanzi?

(Note this does exist for a geochemical condition with Alquimia, I'm talking about non-reactive transport.)

@lipnikov
Copy link
Contributor

lipnikov commented Mar 2, 2021

TransportDomainFunction is a simple base class for PK_DomainDunctionXXX classes that implement various source models. Mathematically we have only one Q in dC/dT + div (v C) = Q and it is in [mol/m3/s]. My understanding of what you are trying to achieve is to calculate Q as a function of other input data, e.g. Q = Q(solute_mass, water_mass, water_flux, etc). Often this is just a post-processing step (after calculating data using a source function) and could be delegated to a sub-model. In Amanzi, sub-models are supported by the base class. Since TransportDomainFunction is almost empty it is possible either to extend it or to implement a separate base class in ATS's transport.

Transport PK in Amanzi has one example of pumping out just water. Unfortunately, it is not yet implemented as a sub-model, so examples of using submodes are in Flow PK only.

@ecoon ecoon added this to the ATS 1.1 milestone Mar 23, 2021
@ecoon ecoon closed this as completed in #91 May 4, 2021
ecoon added a commit that referenced this issue May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants