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

We do not support the 3rd degenerate case for Tori #156

Open
makeclean opened this issue Jun 6, 2024 · 4 comments · May be fixed by #157
Open

We do not support the 3rd degenerate case for Tori #156

makeclean opened this issue Jun 6, 2024 · 4 comments · May be fixed by #157

Comments

@makeclean
Copy link
Contributor

The mcnp manual states there are three 'kinds' of torus;

  1. major radius > minor radius and major radius > 0 (normal torus)
  2. major radius < minor radius and major radius > 0 (degenerate torus where there is some overlap forming a D shape)
  3. major radius < minor radius and major radius < 0 (degenerate torus where there is some overlap forming a lens shape)

It seems we do not support the third use case.

@gonuke
Copy link
Member

gonuke commented Jun 7, 2024

For some reason I didn't think we supported either one. In case 2, do we actually get some kind of "infinity" symbol/ Venn diagram shape?

@gonuke
Copy link
Member

gonuke commented Jun 7, 2024

What solutions are on the table?

  1. identify and report an error
  2. improve documentation
  3. enhance to code to support this case

I suppose definitely 2 and either 1 or 3?

@MicahGale
Copy link

MicahGale commented Jun 7, 2024

1 vs. 3 sounds like a design philosophy question:

Ought this plugin support all valid (at least according to documentation) MCNP input or not?

I think no matter the answer to that question the short term solution should be pursuing 1 and 2 as those seem the lowest effort, and 3 should be a long term feature addition (if ever).

My two cents on support for this edge case: this seems to be a lot of effort to support for an edge case, and I don't think it's worth it. Though I think a huge part of my opinion on this is based on the assumption that all forms of case 3 lenticular shapes can be replicated with the intersection of two ellipsoids or parabloids.

@gonuke
Copy link
Member

gonuke commented Jun 8, 2024

I just realized that this should actually live in the mcnp2cad repo

This does belong here - because it's implemented in the lightweight iGeom we made for this purpose

@gonuke gonuke linked a pull request Jun 8, 2024 that will close this issue
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.

3 participants