Skip to content

Revert incorrect naming changes - ExpSphere is the correct model name#203

Merged
The9Cat merged 7 commits intoSphericalExactfrom
copilot/sub-pr-199-another-one
Feb 4, 2026
Merged

Revert incorrect naming changes - ExpSphere is the correct model name#203
The9Cat merged 7 commits intoSphericalExactfrom
copilot/sub-pr-199-another-one

Conversation

Copy link

Copilot AI commented Feb 4, 2026

This PR reverts incorrect changes that were made based on a misunderstanding of feedback on PR #199.

Background:
The original feedback pointed out a documentation inconsistency, not a code issue. The code correctly used ExpSphere as the user-facing model name, while the underlying implementation class is ExpDeproj. The initial changes incorrectly renamed the user-facing model from ExpSphere to ExpDeproj.

Changes:

  • Reverted enum value from ExpDeproj back to ExpSphere in EmpCylSL.H
  • Reverted default config value and string comparison back to "ExpSphere" in BiorthBasis.cc
  • Reverted enum mappings and switch cases back to ExpSphere in EmpCylSL.cc

Current state (correct):

  • User-facing model name: ExpSphere (what users specify in YAML configs)
  • Internal implementation: ExpDeproj class (C++ class providing the mathematical implementation)

Users should use ExpSphere (case-insensitive) in YAML configuration:

basis:
  mtype: ExpSphere  # correct model name

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits February 4, 2026 15:11
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Copilot AI changed the title [WIP] Update spherical basis for deprojection of density Align model naming from ExpSphere to ExpDeproj Feb 4, 2026
Copilot AI requested a review from The9Cat February 4, 2026 15:13
@The9Cat
Copy link
Member

The9Cat commented Feb 4, 2026

@copilot I believe that the correct tag for this is ExpSphere not ExpDeproj. It was changed from ExpDeproj to ExpSphere in an earlier commit. Can you check that again?

Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Copy link
Author

Copilot AI commented Feb 4, 2026

@copilot I believe that the correct tag for this is ExpSphere not ExpDeproj. It was changed from ExpDeproj to ExpSphere in an earlier commit. Can you check that again?

You're absolutely right - I misunderstood the original feedback. ExpSphere is the correct user-facing name for the model. I've reverted all the changes in commit 7f723ba.

The code now correctly uses:

  • ExpSphere as the enum value and user-facing model name (what users put in YAML configs)
  • ExpDeproj as the internal C++ class name for the implementation

Users should use ExpSphere (or expsphere case-insensitive) in their YAML configuration files.

Copilot AI and others added 2 commits February 4, 2026 15:26
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Copilot AI changed the title Align model naming from ExpSphere to ExpDeproj Revert incorrect naming changes - ExpSphere is the correct model name Feb 4, 2026
@The9Cat The9Cat marked this pull request as ready for review February 4, 2026 15:29
@The9Cat The9Cat merged commit d227415 into SphericalExact Feb 4, 2026
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 this pull request may close these issues.

2 participants