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

Inconsistent input validation (.xinp files) regarding the "value"-attribute #101

Open
VikingScientist opened this issue Jan 16, 2017 · 5 comments

Comments

@VikingScientist
Copy link
Contributor

VikingScientist commented Jan 16, 2017

There are essentially 4 ways of specifying parameters, here illustrated with the "mode" parameter to the "eigensolver" tag.

<simulation>
  <eigensolver mode="1">
    <mode value="2">   3    </mode>
  </eigensolver>
</simulation>

and run this with the commandline parameter <myProgram> <myInputFile> -eig 4. The current ordering is that commandline takes precedence over .xinp-file parameters, and then it goes in decreasing order 3,2 and finally mode="1"which has the lowest priority. There is however varied support for this structure. For instance

<simulation>
  <discretization nGauss="1">
    <nGauss value="2">   3    </nGauss>
  </discretization>
</simulation>

only supports version 3 while

<simulation>
  <adaptive maxstep="1">
    <maxstep value="2">   3    </maxstep>
  </adaptive>
</simulation>

only supports version 2 and 3.

I'm trying to generate some .xinp-files (file generator) and was wondering what is "best practice" and what is the officially supported variants that we want to support. At the very least, I think the <nGauss>-case above should suffice as a bug, since I think version 2 should be supported.

@kmokstad
Copy link
Contributor

kmokstad commented Jan 16, 2017

The two latter examples above are malformed.
Guess you ment to write <nGauss value="2">3</nGauss> and <maxstep value="2">3</maxstep> ?

Then I can say that in general both of these variants are supported, and the value="#" form will override the other form, if both are specified. This depends on whether the method utl::getValue is being used (see Utilities.C lines 274-292).

nGauss is an exception here, where utl::getValue is not used (and therefore a value="#" form is ignored). That is because you here you can specify two values, like <nGauss>2 3</nGauss>, meaning that a 2-point rule is to be used when integrating the tangent and residual, wheres a 3-point rule is to be used in post-processing tasks (GL2-recovery and norm integration). I thought this was already documented on the IFEM wiki?

@VikingScientist
Copy link
Contributor Author

The two latter examples above are malformed.
Guess you ment to write 3 and 3 ?

Yes, you are quite right. Updated OP to the correct version now.

Then I can say that in general both of these variants are supported, and the value="#" form will override the other form, if both are specified

Whoops, my bad. The correct order of preference when all 4 versions is supported is 4,2,3,1 as you said.

The reason I brought this up was because I wanted a discussion around what is supported and what is recommended .xinp syntax. For the nGauss case, there is nothing wrong from an xml or c++ point of view in supporting the following syntax

<simulation>
  <discretization nGauss="2 3">
    <nGauss value="2 3">   2 3  </nGauss>
  </discretization>
</simulation>

But I do get that it is annoying to have to write utl::getValue-function for every possible variation of int/float/string/array/bool. Could we establish the following guidelines:

  • <nGauss> 2 3 </nGauss> is the recommended form
  • <nGauss value="2 3"> </nGauss> is supported wherever it makes sense, but is considered a hidden feature
  • <discretization nGauss="2 3"> is discontinued and all tests in the official repositories using this should be updated (but you are free to use it in your local .xinp files)

@kmokstad
Copy link
Contributor

The utl::getValue function is supposed to be used only when there is a single value. Therefore it is not used in the nGauss case. Morever, the form <discretization nGauss="2 3"> has neven been there for nGauss (and will never be). There are only a few tags (like <eigensolver> and <timestepping>) which support specification of parameters both as attributes on the tag itself, and via sub-tags. When it comes to writing xml-generators, I would stay away from these special cases and stick with the <tag>(value)</tag>form, which always should work.

@VikingScientist
Copy link
Contributor Author

The thing is that I started to write an XML schema, or .xsd file. The basic idea was that this should serve triple purpose

  1. Used to generate a point & click GUI for automatic .xinp-generation
  2. XML verification; which is XML schemas primary function anyway
  3. Documentation - a quick lookup on what valid tags & values we have for those that prefer ASCII-files to the GUI

It was during this process that I discovered that multiple ways of writing the same thing is hideously complex from an xml point of view. While I can accomplish point 2 in the 3-point list above, there is no way that the same file can be useful for point 1 and 3. I think I should be able to conjure up an xsd-file which only allows the <tag>(something)</tag>, and even go as far as <tag value="(something)"/> if (something) is a simple type: int/string/bool; i.e. not arrays or enums. However, in order to make the same file applicable for XML verification (point 2 above), then we need to update the test farm since it currently is using a lot of exotic syntax. The .xsd-file should be able to correctly accept any .xinp-file in the official repository. This would mean rewriting a lot of the tests to the recommended format of <tag>(something)</tag> as many places as possible.

As an added bonus, I think this will generate a lot nicer input-files all across the board, seeing as the current work-flow for many scientists is to copy-paste an existing .xinp-file and tweak the parameters to suite their slightly different application. If the original file is pretty, then every copy will be pretty 😄

@VikingScientist
Copy link
Contributor Author

Just noticed that AdvectionDiffusion is using <stabilization type="supg"/>, which adds another syntax for specifying a simple input field.

Since we are getting attention from users I think this would be a good time to look into standardizing and increasing the user-friendliness of the framework.

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

No branches or pull requests

2 participants