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

Improving ModelArrayRef #249

Closed
timspainNERSC opened this issue Mar 1, 2023 · 2 comments · Fixed by #250
Closed

Improving ModelArrayRef #249

timspainNERSC opened this issue Mar 1, 2023 · 2 comments · Fixed by #250
Assignees
Labels
enhancement New feature or request

Comments

@timspainNERSC
Copy link
Collaborator

Currently ModelArrayRef has two outstanding problems:

  1. The set of possible referents is limited to whatever is listed in the ProtectedArray and SharedArray enums.
  2. There is an additional dereference to get the position of the referenced array in the big array of pointers.

The main issue with problem 1 is that it limits the ability of future researchers to add new fields that can be shared by the ModelArrayRef mechanism. If a future researcher wishes to add young ice parameters, then they would have to add HICE_YOUNG, CICE_YOUNG &c. to the enum in order to use the ModelArrayRef class to share the data throughout the model.

If the key used to access the referenced data was a string, then the value could be much more arbitrary, albeit at the risk of name clashes. If the key is being changed to a string, then since the underlying store is to be changed, the additional dereference could be eliminated.

@timspainNERSC timspainNERSC added the enhancement New feature or request label Mar 1, 2023
@timspainNERSC timspainNERSC added this to the 1 Dynamics merge milestone Mar 1, 2023
@timspainNERSC timspainNERSC self-assigned this Mar 1, 2023
@timspainNERSC timspainNERSC moved this from Todo to In Progress in neXtSIM_DG overview Mar 1, 2023
@timspainNERSC
Copy link
Collaborator Author

The code to fix this is present in the branches containing the substring mar3 and their descendants.

@timspainNERSC timspainNERSC linked a pull request Aug 28, 2023 that will close this issue
timspainNERSC added a commit that referenced this issue Oct 27, 2023
@timspainNERSC
Copy link
Collaborator Author

Closed by merging PR #250

@github-project-automation github-project-automation bot moved this from In Progress to Done in neXtSIM_DG overview Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

2 participants