-
Notifications
You must be signed in to change notification settings - Fork 145
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
explicitly instantiate CrystalLattice simplify initialization #5341
Conversation
add accessors to facilitate encapsulation and full precision lattices
@@ -23,6 +23,7 @@ | |||
#define OHMMS_CRYSTALLATTICE_H | |||
#include <limits> | |||
#include <iostream> | |||
#include <OhmmsData/Libxml2Doc.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't successfully forward declare xmlNodePtr. This is needed to support the factory function I use instead of the stateful LatticeParser in later PR's.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't mean using forward declare. I cannot find any use of xmlNodePtr under src/Particle/Lattice
and thus feel the actual consumer of xmlNodePtr should have the include line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That causes the include to basically always be included before CrystalLattice.h, I did the right thing and figured out how to foward declare it.
@@ -280,10 +311,27 @@ struct CrystalLattice : public LRBreakupParameters<T, D> | |||
|
|||
//! Print out CrystalLattice Data | |||
void print(std::ostream&, int level = 2) const; | |||
|
|||
// Allow assignment operator between say T=double and T=float | |||
template<class TT, unsigned DD> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the dimension be just D
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't partially specialize a class friend. So as soon as this stops being a struct and we stop direct access to lattice internals. The assignment operator becomes harder to write and dependent on accessors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I forgot...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropped attempt to set ground work to kill .set. For the need for a fully defined xmlNodePtr and a non specialized friend definition, these are both stepping stones. The knowledge of xmlNodePtr will disappear once LatticeInput exists, and the alternate precision friend class can disappear once there are no reduced precision lattices in the code.
@@ -280,10 +311,27 @@ struct CrystalLattice : public LRBreakupParameters<T, D> | |||
|
|||
//! Print out CrystalLattice Data | |||
void print(std::ostream&, int level = 2) const; | |||
|
|||
// Allow assignment operator between say T=double and T=float | |||
template<class TT, unsigned DD> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't partially specialize a class friend. So as soon as this stops being a struct and we stop direct access to lattice internals. The assignment operator becomes harder to write and dependent on accessors.
@@ -23,6 +23,7 @@ | |||
#define OHMMS_CRYSTALLATTICE_H | |||
#include <limits> | |||
#include <iostream> | |||
#include <OhmmsData/Libxml2Doc.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't successfully forward declare xmlNodePtr. This is needed to support the factory function I use instead of the stateful LatticeParser in later PR's.
Test this please |
Test this please |
@ye-luo suggestion this works as a separate PR.
Build and link CrystalLattice once as opposed to repeatedly due to including implementation into header.
Add accessors to facilitate encapsulation and full precision lattices.
this supports #5327
What type(s) of changes does this code introduce?
Delete the items that do not apply
Does this introduce a breaking change?
What systems has this change been tested on?
sdgx server
Checklist
Update the following with a yes where the items apply. If you're unsure about any of them, don't hesitate to ask. This is
simply a reminder of what we are going to look for before merging your code.