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

FATES insert_cohort subroutine does not preserve order of cohorts #1313

Open
adrifoster opened this issue Jan 14, 2025 · 0 comments
Open

FATES insert_cohort subroutine does not preserve order of cohorts #1313

adrifoster opened this issue Jan 14, 2025 · 0 comments

Comments

@adrifoster
Copy link
Contributor

adrifoster commented Jan 14, 2025

The insert_cohort subroutine flips the order of cohorts (given identical heights), while sort_cohorts preserves order.

Essentially, this means that if we give insert_cohort a list of cohorts 1...100 (all the same exact height), the resulting list will be ordered 100...1.

This is because of this line:

    current => pshortest
    exitloop = 0
    !starting with shortest tree on the grid, find tree just
    !taller than tree being considered and return its pointer
    if (associated(current)) then
       do while (associated(current).and.exitloop == 0)
          **if (current%height < tsp) then**
             current => current%taller
          else
             exitloop = 1
          endif
       enddo
    endif

if we switch this to if (current%height <= tsp) then we will preserve order.

I think this is not ideal, and is somewhat confusing. I would like to propose we make this change this so that insert_cohort preserves the order.

I have a branch that updates our insert_cohort and sort_cohort routines. Because of this change, it is not B4B. I also propose adding some checks to make sure we don't have any inadvertently disconnected cohorts.

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

No branches or pull requests

1 participant