add in k vector tests

[Kvieta1990]

No description provided.

[briantoby]

The tests were failing before because seekpath was not being installed; that has been addressed, at least for "smoke test". Also, when seekpath is not found, now tests are skipped and are marked "s" in the output.

Now some of the tests are failing, with these errors (see logs for more details):

FAILED ..\tests\test_kvec.py::test_LatConstruct - AssertionError: Lattice vectors construction failed
assert False
 +  where False = <function allclose at 0x00000168DD3FB430>([np.float64(4.330127018922194), np.float64(-2.499999999999999), np.float64(3.061616997868383e-16)], [4.33013, -2.5, 3.06162], atol=1e-05)
 +    where <function allclose at 0x00000168DD3FB430> = np.allclose

FAILED ..\tests\test_kvec.py::test_KVecCandidateUpdate - TypeError: kVector.updateCandidateList() missing 2 required positional arguments: 'max_dd' and 'try_neg'

FAILED ..\tests\test_kvec.py::test_KVecSearch - TypeError: kVector.__init__() got an unexpected keyword argument 'kscope'

[briantoby]

Also, I don't know if there is a way so that a push to pull requests only generates one set of self tests (https://github.com/AdvancedPhotonSource/GSAS-II/blob/420d89118863f207184e08db084283f7bb18445c/.github/workflows/smoke_test.yml), but if you have advice, I'd be happy to try that.

[Kvieta1990]

The tests were failing before because seekpath was not being installed; that has been addressed, at least for "smoke test". Also, when seekpath is not found, now tests are skipped and are marked "s" in the output.

Now some of the tests are failing, with these errors (see logs for more details):

FAILED ..\tests\test_kvec.py::test_LatConstruct - AssertionError: Lattice vectors construction failed
assert False
 +  where False = <function allclose at 0x00000168DD3FB430>([np.float64(4.330127018922194), np.float64(-2.499999999999999), np.float64(3.061616997868383e-16)], [4.33013, -2.5, 3.06162], atol=1e-05)
 +    where <function allclose at 0x00000168DD3FB430> = np.allclose

FAILED ..\tests\test_kvec.py::test_KVecCandidateUpdate - TypeError: kVector.updateCandidateList() missing 2 required positional arguments: 'max_dd' and 'try_neg'

FAILED ..\tests\test_kvec.py::test_KVecSearch - TypeError: kVector.__init__() got an unexpected keyword argument 'kscope'

I will push some changes to fix this.

[Kvieta1990]

Also, I don't know if there is a way so that a push to pull requests only generates one set of self tests (https://github.com/AdvancedPhotonSource/GSAS-II/blob/420d89118863f207184e08db084283f7bb18445c/.github/workflows/smoke_test.yml), but if you have advice, I'd be happy to try that.

This is the suggested change I got from github copilot,

name: smoke test

on:
  push:
    branches-ignore:
      - master
  pull_request:
    branches: ['main']
    paths-ignore:
      - 'path/to/script/to/ignore/**'
  workflow_dispatch:

where path/to/script/to/ignore/** is supposed to ignore certain files under a directory and I think we can also specify a file or some files that match the pattern to ignore them.

I am not sure whether this will work but maybe we can give it a try?

[Kvieta1990]

I am seeing failures about the kvec_general module not being found. This is the compiled Cython module and I am not sure why it it not pulled in properly.

[briantoby]

I am seeing failures about the kvec_general module not being found. This is the compiled Cython module and I am not sure why it it not pulled in properly.

Good question. It gets compiled as part of the self-test with the pixi install. There are some warnings there: https://github.com/AdvancedPhotonSource/GSAS-II/actions/runs/14004790689/job/39217191633?pr=157#step:4:357. It should be using this meson file: https://github.com/AdvancedPhotonSource/GSAS-II/blob/kvec_test/sources/k_vec_cython/meson.build to build. If you are getting different results, we should take a close look at how you are compiling and linking and compare that to what meson is doing.

[Kvieta1990]

I am seeing failures about the kvec_general module not being found. This is the compiled Cython module and I am not sure why it it not pulled in properly.

Good question. It gets compiled as part of the self-test with the pixi install. There are some warnings there: https://github.com/AdvancedPhotonSource/GSAS-II/actions/runs/14004790689/job/39217191633?pr=157#step:4:357. It should be using this meson file: https://github.com/AdvancedPhotonSource/GSAS-II/blob/kvec_test/sources/k_vec_cython/meson.build to build. If you are getting different results, we should take a close look at how you are compiling and linking and compare that to what meson is doing.

Here is what I do to compile the module locally.

First, I have the source codes in the kvec_general.pyx file. Then I have the setup.py sitting in parallel with the source code file, and here below is my setup.py file,

from setuptools import setup
from Cython.Build import cythonize
import numpy as np

setup(
    ext_modules=cythonize("kvec_general.pyx"),
    include_dirs=[np.get_include()]
)

I have a conda environment with cython installed and next I would activate the conda environment, followed by executing the following compiling command,

python setup.py build_ext --inplace

This will create the compiled module file. On Windows, it ends with .pyd and on MacOS and Linux, it ends with .so.

Maybe this will help identify what the issue is with the compiling in GitHub action? Please feel free to let me know if anything further you need from me. Thanks!

[Kvieta1990]

I think I know what the problem is but not sure about the solution. In GitHub action when compiling the kvec_general cython module with meson, I think the compiled module file stays in the sources/k_vec_cython directory and the k_vector_search.py file cannot locate the compiled cython module kvec_general in this line from kvec_general import parallel_proc.

Do we need to insert sources/k_vec_cython into the system path in k_vector_search.py or do we have other solutions just for the purpose of testing?

[briantoby]

I fixed the import problem (imports are now relative) and added some more detail when errors occur. Also added some code to prevent pytest from failing due to the warning generated by seekpath.

The test is now failing because a different value is being computed:

(py313) py3.13 main % python tests/test_kvec.py 
k_vector_search: kvec_general could not be imported
/Users/toby/py/mf3/envs/py313/lib/python3.13/site-packages/seekpath/hpkot/__init__.py:172: DeprecationWarning: dict interface is deprecated. Use attribute interface instead
  conv_lattice = dataset["std_lattice"]
testing test_kvec.py with test_AtomID
	(self-test #0: test the unique ID generation routine)
test_AtomID passed
testing test_kvec.py with test_LatConstruct
	(self-test #1: test the lattice vectors construction routine)
test_LatConstruct passed
testing test_kvec.py with test_CriticalRoutines
	(self-test #2: test the critical routines)
testing test_kvec.py with test_KVecCandidateUpdate
	(self-test #3: test the updating of the list of alternative k vectors)
Traceback (most recent call last):
  File "/Users/toby/G2/main/tests/test_kvec.py", line 450, in <module>
    test()
    ~~~~^^
  File "/Users/toby/G2/main/tests/test_kvec.py", line 236, in test_KVecCandidateUpdate
    assert np.allclose(
           ~~~~~~~~~~~^
        k_opt_dist[0],
        ^^^^^^^^^^^^^^
        0.14445,
        ^^^^^^^^
        atol=1e-5
        ^^^^^^^^^
    ), msg+f'\nk_opt_dist[0] {k_opt_dist[0]}, 0.14445'
    ^
AssertionError: Test for updating the list of alternative k vectors failed
k_opt_dist[0] 0.08339840674252252, 0.14445

[Kvieta1990]

I fixed the import problem (imports are now relative) and added some more detail when errors occur. Also added some code to prevent pytest from failing due to the warning generated by seekpath.

The test is now failing because a different value is being computed:

(py313) py3.13 main % python tests/test_kvec.py 
k_vector_search: kvec_general could not be imported
/Users/toby/py/mf3/envs/py313/lib/python3.13/site-packages/seekpath/hpkot/__init__.py:172: DeprecationWarning: dict interface is deprecated. Use attribute interface instead
  conv_lattice = dataset["std_lattice"]
testing test_kvec.py with test_AtomID
	(self-test #0: test the unique ID generation routine)
test_AtomID passed
testing test_kvec.py with test_LatConstruct
	(self-test #1: test the lattice vectors construction routine)
test_LatConstruct passed
testing test_kvec.py with test_CriticalRoutines
	(self-test #2: test the critical routines)
testing test_kvec.py with test_KVecCandidateUpdate
	(self-test #3: test the updating of the list of alternative k vectors)
Traceback (most recent call last):
  File "/Users/toby/G2/main/tests/test_kvec.py", line 450, in <module>
    test()
    ~~~~^^
  File "/Users/toby/G2/main/tests/test_kvec.py", line 236, in test_KVecCandidateUpdate
    assert np.allclose(
           ~~~~~~~~~~~^
        k_opt_dist[0],
        ^^^^^^^^^^^^^^
        0.14445,
        ^^^^^^^^
        atol=1e-5
        ^^^^^^^^^
    ), msg+f'\nk_opt_dist[0] {k_opt_dist[0]}, 0.14445'
    ^
AssertionError: Test for updating the list of alternative k vectors failed
k_opt_dist[0] 0.08339840674252252, 0.14445

Thank you so much for your fix! I will take care of the failing test then. Also, maybe next time when we meet, I may need your help show me how to run the tests locally so I can test on my local machine before pushing codes. That will be super helpful. Thanks!

[briantoby]

These tolerances seem pretty large. OTOH if the code runs and produces an answer that is already a pretty good test. I'm OK with this