search

optados-developers / optados

Official Repository of the Optados code
http://www.optados.org
20 stars 22 forks source link

OptaDOS Test Suite -- First Attempt (in branch Example) #48

Closed ajm143 closed 2 years ago

ajm143 commented 2 years ago

PR contains:

Missing:

ajm143 commented 2 years ago

Good! I wanted to see fpretify pre-commit work!

jryates commented 2 years ago

Test suite passes fine with gfortran. With intel I'm getting a failure with the testopt_optics_conductivity which I think is due to using a too strict absolute error.

tests/testopt_optics_conductivity - Al4.odi: **FAILED**.
170final_element_two
    ERROR: absolute error 1.16e-10 greater than 1.00e-10. (Test: 925767.491625.  Benchmark: 925767.491625.)
330final_element_two
    ERROR: absolute error 1.46e-10 greater than 1.00e-10. (Test: 202350.49094.  Benchmark: 202350.49094.)
290final_element_two
    ERROR: absolute error 4.07e-10 greater than 1.00e-10. (Test: 506424.954013.  Benchmark: 506424.954013.)
jryates commented 2 years ago

Should make default to making optados.x and od2od.x? If you attempt to run the test suite without od2od.x being there, the errors are a bit alarming.

ajm143 commented 2 years ago

1) I'm happy to modify the default errors for tests/testopt_optics_conductivity, or you can if you're already "in the zone". 2) Also happy for make to default to making both. I've done this for the GitHub actions -- but I agree, it does look a bit catastrophic and not obvious how to solve.

jryates commented 2 years ago

@ajm143 I see what you have done with this latest check in - the parser now returns a fixed number of lines. To set the tolerances do I need something like the following - or is there a better way? (it passes when I make this change, but I will continue to test various permutations)

tolerance = ( (1.0e-5, 1.0e-5, 'l10_c2'), (1.0e-5, 1.0e-5, 'l20_c2'), (1.0e-5, 1.0e-5, 'l30_c2'), (1.0e-5, 1.0e-5, 'l40_c2'), (1.0e-5, 1.0e-5, 'l50_c2'), (1.0e-5, 1.0e-5, 'l60_c2'), (1.0e-5, 1.0e-5, 'l70_c2') )

ajm143 commented 2 years ago

Sorry, I shouldn't have added to that to the PR yet! I'll add your tolerance snippet and test.

Currently working my way through TCM's compilers.

  1. PR currently won't work with the NAG compiler because it has issues with od2od.
    • needs '1x' not 'x' in format string (it's right!)
    • complaining about overfilling the read-in buffer of formatted ome elements. I can slightly tweak the read and write of formatted ome's (remove the implicit do loops) but that might require remaking the /checkpoints -- joy.
  2. g95 I think is dead -- and we should remove (just not in this PR.)
  3. Pgi is rarely painless, but will give it a go.
ajm143 commented 2 years ago

I think the tests run ok on gfortran / ifort / oneAPI / AOCC and pgi. I couldn't find a modern Sun compiler, and NAG is quite picky about the read statements in od2od. @jryates is it worth getting NAG to be happy ?

jryates commented 2 years ago

If I understand correctly, NAG isn't finding a fortran issue - it's failing due to some internal limit in its I/o buffers. This is presumably connected to formatted IO (not unformatted IO). Any chance there is environment variable or compiler option that controls this? NAG is generally a good compiler for a test suite. So might be worth a bit of effort. For unformatted IO it is crucial to use the nested loops (speed and size of file on disk). I'm not sure how much the benefit translates to formatted IO (my guess is it matters less).

ajm143 commented 2 years ago

Thanks -- this was a helpful spot. This isn't as obvious as it ought to be to solve from the error message.

In case you have to come this way again: it is as you say. recl in the OPEN statement sets the maximum record length. If it is not set the compiler uses a default. For gfortran the default is 1073741824 bytes (1 GB), and can be set by the environment variable GFORTRAN_DEFAULT_RECL. For NAG, I couldn't find any environment variables or CL options that looked helpful. From the manual the "Default maximum record length for formatted output = 1024 characters" Where I don't have a great definition of "character", (but they're unlikely to be > 1MB each!)

Setting recl=1073741824 in theopen statement satisfied NAG and and can do no harm to the >90% of users that were using gfortran, hence that value as their default anyway.

I agree on both NAG being a useful testing compiler and on your thoughts regarding loops. Apart from a few more x=>1x in the format statements to squash, I think we can have a working distro* based on NAG.

*autocorrect preferred "bistro".

ajm143 commented 2 years ago

@jryates I think this is ready for you destructively test again!

jryates commented 2 years ago

Just picking a test at random

Running test using /home/yates/CODE/OPT/optados/optados/test-suite/tests/../../optados.x Si2.odi > test.out.02032022-1.inp=Si2.odi 2> test.err.02032022-1.inp=Si2.odi in /home/yates/CODE/OPT/optados/optados/test-suite/tests/testopt_core_all

Analysing output using function parse in /home/yates/CODE/OPT/optados/optados/test-suite/tests/testopt_core_all on files benchmark.out.default.inp=Si2.odi and test.out.02032022-1.inp=Si2.odi.

[parsers.parse_od2od_fmt.parse] Parsing file 'benchmark.out.default.inp=Si2.odi'

l10_c2: [] l20_c2: [0.0] l30_c2: [0.0] l40_c2: [0.0] l50_c2: [0.0] l60_c2: [0.0] l70_c2: [0.0]

[parsers.parse_od2od_fmt.parse] Parsing file 'test.out.02032022-1.inp=Si2.odi' l10_c2: [] l20_c2: [0.0] l30_c2: [0.0] l40_c2: [0.0] l50_c2: [0.0] l60_c2: [0.0] l70_c2: [0.0]

Passed. l20_c2 l30_c2 l40_c2 l50_c2 l60_c2 l70_c2 benchmark 0.0 0.0 0.0 0.0 0.0 0.0
test 0.0 0.0 0.0 0.0 0.0 0.0

All done. 1 out of 1 test passed.

So it is testing on the low energy part of the spectrum - which is not broadened - so we are just getting zero.

I'm not sure what the best approach is here. Perhaps we can be more specific about the energy ranges - so that the first few entries have meaningful values?

ajm143 commented 2 years ago

Hmm. I've been around in a circle on this one. Perhaps I need to work a bit harder -- how about instead of sampling the first 70 lines? We sample 10 (say) equally spaced lines throughout the file... ?

For example 99b6291 is better -- but not perfect

jryates commented 2 years ago

I suppose we might do both: In example 8 setting

DOS_SPACING : 1 DOS_MIN_ENERGY : -3 DOS_MAX_ENERGY : 7

Prints out only a few numbers - but they are all meaningful to test on. It also speeds up the test. But there are multiple edges to test on in the file - and it would need a little bit of care to deal with this.

The one issue with testing quite a lot of data is that the current approach gives each data element an individual tag - and so we need to set the tols for each tag. Either there needs to be a regex match, or we use the same tag.

ajm143 commented 2 years ago

Bit confused by your last comment: 1) Re: 8. I might have to admit defeat and make more specific parsers ? I think that would allow much more control. I can do this but I'm confused about: 2) Individual tags: not sure I'm understanding what you think is better here? Can we have multiple data in one tag? (Have I missed a feature?)

jryates commented 2 years ago

If you skip the line counter from the parser - so

retdict["c2"].append(float(elements[1]))

Then I think it makes an array - so we can compare a set in one go.

Analysing output using function parse in /home/yates/CODE/OPT/optados/optados/test-suite/tests/testopt_core_all on files benchmark.out.default.inp=Si2.odi and test.out.02032022-4.inp=Si2.odi. [parsers.parse_od2od_fmt.parse] Parsing file 'benchmark.out.default.inp=Si2.odi' c2: [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]

[parsers.parse_od2od_fmt.parse] Parsing file 'test.out.02032022-4.inp=Si2.odi' c2: [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]

Passed. c2 c2 c2 c2 c2 c2 benchmark 0.0 0.0 0.0 0.0 0.0 0.0 test 0.0 0.0 0.0 0.0 0.0 0.0

All done. 1 out of 1 test passed.

ajm143 commented 2 years ago

I think 6d2827a is better.