cyjoelchen / php-sweph

PHP Extension for Swiss Ephemeris
Other
58 stars 27 forks source link

Add documentation & tests. #22

Closed kevindecapite closed 3 years ago

kevindecapite commented 3 years ago

Instructions:


@aloistr wrote:

we should have test cases, so that we could run 'make test' in a meaningful way.

AstroSign-Dev commented 3 years ago

Hi Kevin

As I have written already many lines of PHP code for Jyotish on my website, just send a note if I can do something to help with testing. AFAIK, and have compiled and tested with PHP8, at the moment there are no errors on PHP8, other than the already known and mentioned ArgInfo and Macros.

Cheers Werner

Am 16.07.2021 um 20:36 schrieb Kevin DeCapite @.***>:



This extension needs test as it's used in production environments.

@aloistrhttps://github.com/aloistr wrote:

we should have test cases, so that we could run 'make test' in a meaningful way.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/cyjoelchen/php-sweph/issues/22, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AHRGWP52RBJDJBUQ5T2OVF3TYB3ZXANCNFSM5AQB7VKA.

kevindecapite commented 3 years ago

@AstroSign-Dev Thank you, Werner. Have a look at writing extension tests here:

https://www.zend.com/running-php-extension-tests

This section is of particular interest to us:

--TEST--
test_test2() Basic test
--SKIPIF--
<?php
if (!extension_loaded(‘test’)) {
    echo ‘skip’;
}
?>
--FILE--
<?php
var_dump(test_test2());
var_dump(test_test2(‘PHP’));
?>
--EXPECT--
string(11) “Hello World”
string(9) “Hello PHP”

I think, ideally, we would execute each SE function within the extension to be thorough. We don't need to check for data accuracy, per se, as that's tested by the SE authors of course. However, to make sure the functions execute properly, take the appropriate parameters, and thus return meaningful data, we will likely need to use "real-world" data for both the --FILE-- and --EXPECT-- sections.

I am hoping to make time for this over the weekend. If you or I get things started, we'll have a pattern which we can repeat easily and perhaps share the workload for the rest.

aloistr commented 3 years ago

I get insignificant diffs like more tests/swe_houses_armc_ex2.diff 057+ float(6.947912832876E-310) 057- float(0) Two problems:

  1. parameters for test cases should be selected realistically, not swe_houses_armc_ex2(0, 0, 23.4, 'P'); but something like swe_houses_armc_ex2(112.65, 43.7, 23.4, 'P')

  2. values of doubles should be compared with limited precision. The two numbers above are both zero, effectively. var_dump as test comparison will often produce issues. See also https://stackoverflow.com/questions/23734857/floating-point-test-assertion-why-do-these-identical-arrays-fail

Hopefully the issue goes away with 1)

kevindecapite commented 3 years ago
  1. Originally, I was on the fence about using more real-world values, and decided to keep it as simple as possible. However, I will change in the tests per your suggestion. Some trivia: 0º, 0º is a named location known as "Null Island"...

https://en.wikipedia.org/wiki/Null_Island

  1. I was wondering if this would happen at some point due to PHP's default precision setting of 14 and knowing that all systems will yield slightly different results at such high precision. I will change the tests to round. Since you are the mathematician and physicist here, what level of precision do you recommend we use?

PS: We could also use the BCMath extension but I think that's unnecessary as simple rounding will do the trick. We are not needing to test the precision of the SE code of course:

https://www.php.net/manual/en/intro.bc.php

kevindecapite commented 3 years ago

Have a look at this PHP code for another example of floating-point troubles, when one doesn't round nor use the proper library:

 $x = 359.99999735188;
 $y = 359.99999735188;

 # bool(true)
 var_dump($x === $y);

 $x = 359.99999735188;
 $y = -0.0000026481173536953 + 360;

 # bool(false), even though $y is 359.99999735188.
 var_dump($x === $y);
AstroSign-Dev commented 3 years ago

I have read in a book of astrology calculations that rounded to 8 digits would be enough for all astrology calculations, but I do not know how many digits are needed for astronomical calculations

There are now some ephemeris files in the root folder, I suggest to put it in an extra folder: eph or similiar

kevindecapite commented 3 years ago

I suggest to put it in an extra folder: eph or similiar

Good suggestion. I would recommend a /data directory.

AstroSign-Dev commented 3 years ago

Yes, data folder is ok, and that below the sweph folder even better :-)

aloistr commented 3 years ago

the problem is, that the default ephe path contains '.' which means 'current directory'. It seems that tests run in the main directory. If the ephe files are there, they are found. If all tests call swe_set_ephe_path('./data') then it should work in a subdirectory data. I suggest to place them in directory sweph, which already exists, and where other original files from the SE distribution live, src and doc. or in sweph/ephe that will need swe_set_ephe_path('./sweph/ephe')

aloistr commented 3 years ago

The directory name 'ephe' has been uses in SE since the beginning, we have all files in /home/ephe

aloistr commented 3 years ago

I suggest we move the se*18.se1 and sefstars.txt into ./sweph/ephe and update all tests to include swe_set_ephe_path('./sweph/ephe') I also suggest to not use SEFLG_MOSEPH in any test, but use SEFLG_SWIEPH. Where appropriate, the flag should also contain SEFLG_SPEED for all functions which have speed in the result vector.

Moshier mode is deprecated, it creates problems of consistency because of a different nutaiton model.