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.

Sign up for GitHub

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

Jump to bottom

New version Swiss ephermeris code #59

Closed
vreijs opened this issue Sep 9, 2020 · 19 comments · Fixed by #60
Closed

New version Swiss ephermeris code #59

vreijs opened this issue Sep 9, 2020 · 19 comments · Fixed by #60

Comments

@vreijs
Copy link
Collaborator

vreijs commented Sep 9, 2020

Hello Ralf.

Here is the code of 2.09.03: https://www.astro.com/ftp/swisseph/
and
https://www.astro.com/ftp/swisseph/src/

There are two new functions to include in the R code:
swe_houses_ex2() and
swe_houses_armc_ex2().
We propose to remove the functions that provide lesser output: swe_houses_ex() and swe_houses_armc()

Let me know when you have time, as I need to replace these two old ones with new ones.

All the best and stay healthy.

Victor

Last comment on this version:

Hi,

this release has three minor bug fixes:

  • An initialization *serr = '\0'; was missing in function swe_calc(), which could lead to crashes when error messages were written.

  • Sidereal positions of asteroids were wrong with ayanamshas 9-16, 21-26, 37, 38, 41, 42. (Namely, all ayanamshas whose initial date is given in UT.)

  • Asteroids with ipl > 10000 (SE_AST_OFFSET): calculating with several different ayanamshas after each other did not work properly.

The first bug was intoduced with version 2.09.01.
The 2nd one exists since version 2.05, this is 5 years old.
The 3rd one exists since more than 20 years. It seems that either sidereal astrologers rarely use asteroids or they rarely use the above-mentioned ayanamshas.

Best wishes,

Dieter
@rstub
Copy link
Owner

rstub commented May 1, 2021

@vreijs Sorry for the delay. I see there is meanwhile version 2.10 available. I guess it makes more sense to use that, right?

@vreijs
Copy link
Collaborator Author

vreijs commented May 1, 2021 via email

@rstub
Copy link
Owner

rstub commented May 3, 2021

Let me know if you have any questions. BTW, with v2.10 I have the following failures in the tests:

══ Failed tests ════════════════════════════════════════════════════════════════
── Failure (test-misc.R:22:5): deltat can be retrieved ─────────────────────────
swe_deltat(1234.567) not equal to 1.58738640540236.
1/1 mismatches
[1] 1.6 - 1.59 == 0.0103
── Failure (test-misc.R:29:5): deltat can be retrieved for vector ──────────────
swe_deltat(c(1234.567, 1234567)) not equal to c(1.5873864, 0.366039979375346).
2/2 mismatches (average diff: 0.0064)
[1] 1.598 - 1.587 == 0.0103
[2] 0.369 - 0.366 == 0.0025
── Failure (test-misc.R:72:5): version works ───────────────────────────────────
swe_version() not equal to "2.08".
1/1 mismatches
x[1]: "2.10"
y[1]: "2.08"
── Failure (test-misc.R:86:3): Determing ayanamsa using UT: ────────────────────
result$daya not equal to 24.99688.
1/1 mismatches
[1] 25 - 25 == -0.000115
── Failure (test-misc.R:94:3): Determing ayanamsa using ET: ────────────────────
result$daya not equal to 24.99688.
1/1 mismatches
[1] 25 - 25 == -0.000115

The first two differences are probably due to a deltaT update, right? The version change is also expected. Any idea about the last two changes?

@rstub rstub mentioned this issue May 3, 2021
Merged
@vreijs
Copy link
Collaborator Author

vreijs commented May 3, 2021 via email

@rstub
Copy link
Owner

rstub commented May 3, 2021 via email

@vreijs
Copy link
Collaborator Author

vreijs commented May 3, 2021 via email

@vreijs
Copy link
Collaborator Author

vreijs commented May 3, 2021 via email

@rstub
Copy link
Owner

rstub commented May 4, 2021

I am not 100% sure, but the changes I introduced bring the deltaT tests without swephRdata in line with those with swephRdata installed: d7adf32
So I would like to do some more tests with this to make sure whether this comes from using version 2.10 or swephRdata being (not) present.

As for the compiler warnings:

     libswe/sweph.c:2306:37: warning: ‘%s’ directive writing up to 255 bytes into a region of size 240 [-Wformat-overflow=]
     libswe/sweph.c:2304:40: warning: ‘%s’ directive writing up to 255 bytes into a region of size 237 [-Wformat-overflow=]
     libswe/sweph.c:2302:41: warning: ‘%s’ directive writing up to 255 bytes into a region of size 236 [-Wformat-overflow=]
     libswe/sweph.c:2300:35: warning: ‘%s’ directive writing up to 255 bytes into a region of size between 234 and 235 [-Wformat-overflow=]
     libswe/sweph.c:2298:34: warning: ‘%s’ directive writing up to 255 bytes into a region of size between 235 and 236 [-Wformat-overflow=]
     libswe/sweph.c:2295:38: warning: ‘%s’ directive writing up to 255 bytes into a region of size between 231 and 240 [-Wformat-overflow=]

Yes, it would be helpful to get input from the SE developers. These come from a portion of the code ( https://github.com/rstub/swephR/blob/feature/v2.10/external/src/sweph.c#L2295-L2306) where a new string s is created based on a format and another string sp. Unfortunately, both s and sp have the same max length and could therefor lead to an overflow. I had tried to use snprintf instead of sprintf, but this gives a "truncation" warning. In my current PR, I try to forcefully truncate sp (cf a646c98) which works locally but not in the (new) CI (https://github.com/rstub/swephR/pull/60/checks?check_run_id=2494991210). One alternative would be to replace %s with something like %.200s, which again does work locally. I have not tried this on CI.

@vreijs
Copy link
Collaborator Author

vreijs commented May 4, 2021 via email

@rstub
Copy link
Owner

rstub commented May 4, 2021

I do a difference with v2.08 depending on whether swephRdata is installed or not. I don't see such a difference for v2.10.

v2.08 w/o swephRdata

ralf@barra:~/tmp$ R_LIBS_USER=~/tmp/R1 R -q -f  swe-test.R 
> library(swephR)
> swe_version()
[1] "2.08"
> requireNamespace("swephRdata")
Loading required namespace: swephRdata
Failed with error:  ‘there is no package called ‘swephRdata’’
> swe_deltat(c(1234.567, 1234567))
[1] 1.587386 0.366040

v2.08 w/ swephRdata

ralf@barra:~/tmp$ R_LIBS_USER=~/tmp/R2 R -q -f  swe-test.R 
> library(swephR)
> swe_version()
[1] "2.08"
> requireNamespace("swephRdata")
> swe_deltat(c(1234.567, 1234567))
[1] 1.5976757 0.3685434

v2.10 w/o swephRdata

ralf@barra:~/tmp$ R_LIBS_USER=~/tmp/R3 R -q -f  swe-test.R 
> library(swephR)
> swe_version()
[1] "2.10"
> requireNamespace("swephRdata")
Loading required namespace: swephRdata
Failed with error:  ‘there is no package called ‘swephRdata’’
> swe_deltat(c(1234.567, 1234567))
[1] 1.5976757 0.3685434

v2.10 w/o swephRdata

ralf@barra:~/tmp$ R -q -f  swe-test.R 
> library(swephR)
> swe_version()
[1] "2.10"
> requireNamespace("swephRdata")
> swe_deltat(c(1234.567, 1234567))
[1] 1.5976757 0.3685434

What would be the expected behavior here?

BTW, thanks for forwarding the suggestion from the SE developers, which does indeed work, cf https://github.com/rstub/swephR/pull/60/checks?check_run_id=2502228436

@vreijs
Copy link
Collaborator Author

vreijs commented May 4, 2021 via email

@rstub
Copy link
Owner

rstub commented May 13, 2021

Using swe_deltat_ex in the four different environments one can learn more about what is going on.

v2.08 w/o swephRdata

ralf@barra:~/tmp$ R_LIBS_USER=~/tmp/R1 R -q -f  swe-test2.R 
> library(swephR)
> swe_version()
[1] "2.08"
> requireNamespace("swephRdata")
Loading required namespace: swephRdata
Failed with error:  ‘there is no package called ‘swephRdata’’
> swe_deltat_ex(c(1234.567, 1234567), 2)
$deltat
[1] 1.587386 0.366040

$serr
[1] "SwissEph file 'seplm48.se1' not found in PATH '/usr/local/lib/R/site-library/swephR/ephemeris/'"                       
[2] "SwissEph file 'seplm18.se1' not found in PATH '/usr/local/lib/R/site-library/swephR/ephemeris/' \nusing Moshier eph.; "

> swe_deltat_ex(c(1234.567, 1234567), 4)
$deltat
[1] 1.587386 0.366040

$serr
[1] "" ""

v2.08 w/ swephRdata

ralf@barra:~/tmp$ R_LIBS_USER=~/tmp/R2 R -q -f  swe-test2.R 
> library(swephR)
> swe_version()
[1] "2.08"
> requireNamespace("swephRdata")
> swe_deltat_ex(c(1234.567, 1234567), 2)
$deltat
[1] 1.5976757 0.3685434

$serr
[1] "" ""

> swe_deltat_ex(c(1234.567, 1234567), 4)
$deltat
[1] 1.587386 0.366040

$serr
[1] "" ""

v2.10 w/o swephRdata

ralf@barra:~/tmp$ R_LIBS_USER=~/tmp/R3 R -q -f  swe-test2.R 
> library(swephR)
> swe_version()
[1] "2.10"
> requireNamespace("swephRdata")
Loading required namespace: swephRdata
Failed with error:  ‘there is no package called ‘swephRdata’’
> swe_deltat_ex(c(1234.567, 1234567), 2)
$deltat
[1] 1.5976757 0.3685434

$serr
[1] "" ""

> swe_deltat_ex(c(1234.567, 1234567), 4)
$deltat
[1] 1.587386 0.366040

$serr
[1] "" ""

v2.10 w/ swephRdata

ralf@barra:~/tmp$ R -q -f  swe-test2.R 
> library(swephR)
> swe_version()
[1] "2.10"
> requireNamespace("swephRdata")
> swe_deltat_ex(c(1234.567, 1234567), 2)
$deltat
[1] 1.5976757 0.3685434

$serr
[1] "" ""

> swe_deltat_ex(c(1234.567, 1234567), 4)
$deltat
[1] 1.587386 0.366040

$serr
[1] "" ""

So it seems like v2.10 no longer needs the SE data files for making deltaT computations using SE instead of Moshier. Is that expected? Or am I misinterpreting things here?

ea6546e also contains the removal of my previous attempt at fixing the new issue. The best reference are allways the files in external/patch, in this case https://github.com/rstub/swephR/blob/ea6546eb004ff448cfac4951a9b3790222e884a0/external/patch/19_increase_buffer.diff.

BTW, thanks for telling me about v2.10.1. I will integrate this into this branch. Can you compile a list of changes since v2.08? I would like to add that to NEWS.md.

@rstub
Copy link
Owner

rstub commented May 13, 2021

BTW, v2.10.1 brings in quite some changes, given that it is "only" a patch release:

  • two source files are no longer used (swepcalc.c and swepdate.c)
  • all other sources are now covered by AGPL and no longer GPL, even though LICENSE still refers to GPL

THe second point is no big issue for us, since swephR has always been covered by AGPL. But might be confusing for other users of the source code. Have the SE authors mentioned this change in any way?

@rstub
Copy link
Owner

rstub commented May 13, 2021

Yet another BTW, the patch 19_increase_buffer.diff is part of version 2.10.01.

@vreijs
Copy link
Collaborator Author

vreijs commented May 13, 2021 via email

@vreijs
Copy link
Collaborator Author

vreijs commented May 13, 2021 via email

@rstub
Copy link
Owner

rstub commented May 13, 2021

Hi Victor,

Concerning AGPL: Excellent! You could ask the SE authors to also update the LICENSE file in their distribution. For us this means we can get rid of the LICENSE file and just say "AGPL" in DESCRIPTION.

Concerning the two no longer used files: They are still part of the distribution (and are therefore still in external/src), but they are no longer needed for building the library. That's why I have deleted them from src/libswe. I see no need for the SE authors to take action, since there are other files in the distribution that are not needed to build the library.

I am curious to see what comes out of the deltaT discussion.

@vreijs
Copy link
Collaborator Author

vreijs commented May 13, 2021 via email

@rstub
Copy link
Owner

rstub commented May 14, 2021

I am refering to the src/LICENSE file in the source distribution:

[ralf@barra: ~/devel/swephR/external] [feature/v2.10 ↑·1|…6⚑ 2] ✔
08:31 $ tar xfO swe_unix_src_2.10.01.tar.gz src/LICENSE | head -n 20
/* Copyright (C) 1997 - 2008 Astrodienst AG, Switzerland.  All rights reserved.

  License conditions
  ------------------

  This file is part of Swiss Ephemeris.

  Swiss Ephemeris is distributed with NO WARRANTY OF ANY KIND.  No author
  or distributor accepts any responsibility for the consequences of using it,
  or for whether it serves any particular purpose or works at all, unless he
  or she says so in writing.  

  Swiss Ephemeris is made available by its authors under a dual licensing
  system. The software developer, who uses any part of Swiss Ephemeris
  in his or her software, must choose between one of the two license models,
  which are
  a) GNU public license version 2 or later
  b) Swiss Ephemeris Professional License

  The choice must be made before the software developer distributes software

@rstub rstub closed this as completed in #60 Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feature/v2.10 rstub/swephR
2 participants
@vreijs @rstub