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

Allow the reordering of a DiscreteDomain #420

Merged
merged 11 commits into from
Apr 22, 2024

Conversation

EmilyBourne
Copy link
Collaborator

@EmilyBourne EmilyBourne commented Apr 19, 2024

Add a function to initialise a DiscreteDomain from a DiscreteDomain with a different ordering. Fixes #419

@tpadioleau
Copy link
Member

I think it should already be addressed by existing constructors. Can you provide an example that does not compile ?

@EmilyBourne
Copy link
Collaborator Author

I think it should already be addressed by existing constructors. Can you provide an example that does not compile ?

Hmm, it seems that you are right. The tests I have added already work on the main branch. The assignment operator doesn't work:

DDomZYX const dom_z_y_x = dom_x_y_z;

but maybe this is intentional?

@tpadioleau
Copy link
Member

I think it should already be addressed by existing constructors. Can you provide an example that does not compile ?

Hmm, it seems that you are right. The tests I have added already work on the main branch. The assignment operator doesn't work:

DDomZYX const dom_z_y_x = dom_x_y_z;

but maybe this is intentional?

I see, I do not see a good reason to not support this assignment operator, it seems to be an equivalent of the reordering & slicing constructor.

@tpadioleau
Copy link
Member

tpadioleau commented Apr 22, 2024

Actually this syntax is not related to the assignment operator. It only involves either the constructor or a cast operator.

That being said, I think it is worth having an equivalent assignement operator of the slicing and reordering constructor.

@EmilyBourne EmilyBourne marked this pull request as ready for review April 22, 2024 09:22
Copy link
Member

@tpadioleau tpadioleau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, do we really need both the 2D and 3D versions then ?

include/ddc/discrete_domain.hpp Outdated Show resolved Hide resolved
@tpadioleau tpadioleau force-pushed the 419-discrete-domain-transpose branch from 06e8969 to e722ab8 Compare April 22, 2024 14:31
Co-authored-by: Thomas Padioleau <thomas.padioleau@cea.fr>
@EmilyBourne
Copy link
Collaborator Author

Thanks, do we really need both the 2D and 3D versions then ?

I don't think so, 3D should be sufficient. I will remove the 2D version

@tpadioleau tpadioleau merged commit 9ef820e into main Apr 22, 2024
39 of 44 checks passed
@tpadioleau tpadioleau deleted the 419-discrete-domain-transpose branch April 22, 2024 16:05
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.

Allow the reordering of a DiscreteDomain
2 participants