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

inversion(circle1, circle2) in geometry.asy not always correct #143

Open
stla opened this issue Feb 11, 2020 · 11 comments · May be fixed by #157
Open

inversion(circle1, circle2) in geometry.asy not always correct #143

stla opened this issue Feb 11, 2020 · 11 comments · May be fixed by #157

Comments

@stla
Copy link

stla commented Feb 11, 2020

Hello,

The function inversion(circle1, circle2) in geometry.asy, which intends to return an inversion which swaps the two circles, is not correct for all cases. See this article on my blog for the complete list of cases.

@ivankokan
Copy link
Contributor

ivankokan commented Feb 11, 2020

Which version of Asymptote are you using? There are multiple examples there and none of them given in asy. Can you please refer to specific ones if not providing your asy code for them (which would be the best)?

@stla
Copy link
Author

stla commented Feb 11, 2020

@ivankokan
Copy link
Contributor

Hello @ivankokan

I refer to the code here: https://github.com/vectorgraphics/asymptote/blob/master/base/geometry.asy.

You did not understand me.

  1. Which version of Asymptote are you using?
  2. Please provide the asy code for which you think inversion is not done correctly.
  3. If you are not able to achieve 2., how can you tell something is wrong? Nevertheless, at least refer to specific geometric configuration at your blog for which Asymptote does not work as expected.

@stla
Copy link
Author

stla commented Feb 11, 2020

I'm using Asymptote 2.44, but I commented the code in geometry.asy.

Ok, I will provide an example. Please wait.

@stla
Copy link
Author

stla commented Feb 11, 2020

Here is an example in the case Case when one circle contains the other.

settings.outformat = "pdf";
import geometry;
size(10cm, 0);

circle circ1 = circle((point)(0,0), 3);
circle circ2 = circle((point)(0.7,1), 0.7);

inversion iota = inversion(circ1, circ2, 1);
circle circ = iota*circ1; // should be circ2

write((pair)circ.C); // (1.12608695652174,1.60869565217391) - not correct
write(circ.r); // 0.7 - correct

Actually I think that only the case Case when the circles are outside each other is correct in geometry.asy.

@ivankokan
Copy link
Contributor

Thanks, I will check later today.

@ivankokan
Copy link
Contributor

ivankokan commented Feb 12, 2020

@stla I will first concentrate on the cases where the radii are not equal.

Demo:

import geometry;

size(80mm, 0);

real[] Rs = {0.4, 1.7};
circle cI = circle(origin, 1);

for (real R : Rs) {
  circle[] cRs = { circle((point) (-3, 0), R), circle((point) (-2, 0), R), circle((point) (-1, 0), R), circle((point) (-0.6, 0), R), circle((point) (0, 0), R),
                   circle((point) (0.2, 0), R), circle((point) (1, 0), R), circle((point) (1.2, 0), R), circle((point) (2, 0), R), circle((point) (2.3, 0), R) };

  for (circle cR : cRs) {
    draw(cI, blue);
    draw(cR);
    draw(cI * cR);
    draw(inversion(cR, cI * cR), red + dashed);
    draw(inversion(cI * cR, cR), green + dashed);
    newpage();
  }
}

Expected outcome: (at least) one among red and green circles should be congruent with the blue one, and it does not happen in all cases.

I will try to track the issue and fix the bug ASAP.
inversion.pdf

@ivankokan
Copy link
Contributor

@stla I did the math on the paper, you can expect the fix this week.

@ivankokan
Copy link
Contributor

ivankokan commented Apr 15, 2020

Hi @stla, @johncbowman. I have a question for you on top of fixing things here.

Considering the fact that for given two circles might exist two resulting inversions (corresponding to both internal and external midcircles), it is necessary to change the prototype of the following function:

inversion inversion(circle c1, circle c2, real sgn = 1)

There are at least two options:

  1. Changing return type to inversion[], and always ensuring predefined order in "two inversions" case, e.g. [internal, external].
  2. Not changing return type, i.e. function will always return one inversion: any existing inversion (regardless whether it is internal or external) if there is only one; or the specific one (in "two inversions" case) based on the state of additional (and last) optional argument, e.g. bool preferinternal=true (or preferinternaliftwo).

Option 1 is not backward-compatible (it would change return type) but I do not think that would be a big issue. Option 2 is backward-compatible, and maybe more user-friendly or semantically driven.

Please share your thoughts.

Edit. Additional observation: for some cases there is no inversion at all, e.g. there is no negative inversion for two intersecting congruent circles. Hence, return type will be changed to inversion[].

@charlesstaats
Copy link
Contributor

To improve backwards compatibility, you could also add an implicit cast from inversion[] to inversion that throws an error if applied to an array of length not equal to one. This won't be perfect since implicit casting is not transitive (so if someone is already implicitly casting an inversion to something else, their code will fail), but it could be an improvement.

@johncbowman
Copy link
Member

Yes, I agree. This clearly makes Option 1 the best choice.

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

Successfully merging a pull request may close this issue.

4 participants