added error checking on cylindrical mesh

[hsameer481]

Description

Added an error-checking method for cylindrical_mesh inputs with the same maximum and minimum range values. Added a unit test for invalid bounds of r_grid, phi_grid, and z_grid. The creation of error checking was prompted by the need to enforce logical constraints. This ensures that each grid value represents a correctly sized segment.

Fixes #2933

Checklist

• I have performed a self-review of my own code
• I have followed the style guidelines (https://docs.openmc.org/en/latest/devguide/styleguide.html#python) for Python source files
• I have added tests that prove my fix is effective or that my feature works

[hsameer481]

@shimwell We have made the following changes to the cylindrical mesh based on the #2933 issue. As first-time (Not just OpenMC but also GitHub) contributors, we wanted to ask if you could review and recommend any changes.

[shimwell]

Wow this is some nice input checking, I hadn't though of checking the numbers increase.

It is certainly an improvement on what we have at the moment.

Would the increasing check catch an input like r_grid=[1,2,4,3]

[hsameer481]

@shimwell Thank you for getting back to us! We have committed new changes that account for input checks similar to [1, 2, 4, 3] as well as adding a unit test for this. Does this logic make sense?

[shimwell]

This all looks good to me, there are lots of ways of checking the list is ascending so I guess we want to make sure we have a fast one.

Also I think this is useful enough in multiple places that we could consider making a check_ascending function in this file . and making use of it in a few places throughout the code.

As a user who makes tons of mistakes I'm always keen to see extra input checking and am in favour of the PR and also like the code and tests you have done.

I'm going to defer to @paulromano for the final judgment on this PR to check we have the balance of extra compute with benefit to user correct.

[shimwell]

@hsameer481 what do you think about the code in #2973. The code added to checkvalues.py looks like we can make use of it on your PR.

What do you think should we merge in #2973 first, then sync your fork and make use of those functions on this PR

[hsameer481]

@shimwell that sounds good to us! Since this is the first time we're doing this, is there a specific process for implementing a new merged PR?

[shimwell]

@shimwell that sounds good to us! Since this is the first time we're doing this, is there a specific process for implementing a new merged PR?

Yep, once the PR has been merged (not yet)

1. go to you own fork
2. selected develop branch on the dropdown of branches
3. click "sync fork"
4. go to your local computer terminal and cd into the repository folder
5. git checkout develop
6. git pull
7. git checkout new_cylindrical_mesh
8. git merge develop
9. then you should have access to the new code which was pushed to openmc-dev/openmc:develop on your feature branch. So you could make use of the new check code.

But we must first wait to see the other PR gets merged

[hsameer481]

@shimwell Great! We really appreciate the guidance and will make sure to follow those steps once it's merged!

[paulromano]

I just made some updates on this branch based on the functionality from #2973. Should be good to go now. Thanks!