SupposeNot / RAMP

Research Assistant for Maps and Polytopes
4 stars 0 forks source link

Changes to ARP? #118

Closed CunningGabe closed 2 years ago

CunningGabe commented 2 years ago

At the moment, ARP(stuff) asserts that the result is polytopal, without checking anything. This sometimes gets us into trouble.

Proposal:

  1. Add an abbreviation RefMan for ReflexibleManiplex.
  2. Maybe remove ARP abbreviation? (But keep AbstractRegularPolytope function). This encourages folks to mostly use RefMan.
  3. Make AbstractRegularPolytope check whether the result is polytopal and throw an error if not. Then add AbstractRegularPolytopeNC.
  4. At some point, we want to handle Pre-Maniplexes more gracefully as well. Technically, ReflexbleManiplex sometimes makes things that are only pre-maniplexes. So we'll have to think about how to handle that as well. (One low-key option would be to have ReflexibleManiplex check whether the result is a real maniplex, and if not, then return a PreManiplex object instead. This should be fast to check, compared to checking for polytopality.)

Note: Making these changes will require us to edit a bunch of test files.

SupposeNot commented 2 years ago

I'm happy with keeping the ARP abbreviation, especially if we implement (3). I like RefMan as an abbreviation. Your low-key solution in (4) is good, but needs to be properly documented.

CunningGabe commented 2 years ago

OK, 1-3 are done in f43acee4128cbc469ecb5081382b41a7afe00b5d. I'll paste what I wrote for 4 above into #97