Require named SPOSet #4160
Require named SPOSet #4160
Conversation
The deprecation warning has been printed for quite a few releases. With the change from this PR, those input files will continue to run but with memory penalty. We continue issue warnings to users reminding them to change input style. I have not decided to turn the warning to error. |
There was a problem hiding this comment.
LGTM but please wait for @PDoakORNL
Happy to see the appearance of accessor functions
| PosType twist(0.0); | ||
| OhmmsAttributeSet attrib; | ||
| attrib.add(norb, "size"); | ||
| attrib.add(twist, "twist"); | ||
| attrib.add(spo_object_name, "name"); |
There was a problem hiding this comment.
If id is intended to be an alternate input tag for the spo_object_name it is missing here.
There was a problem hiding this comment.
In a couple of places, name and id are interchangeable. In this PR, I added both because there are existing input files. It is unclear how we would like to move forward. My current thought is to disable "id" support and allow "name" only. Supporting both requires do a lot of extra things, for example trapping errors when "name" and "id" differ.
For now, we need both both. Here is what I will do.
attrib.add(spo_object_name, "name");
attrib.add(spo_object_name, "id", {}, TagStatus::DEPRECATED)
and later change to TagStatus::DELETED.
I will do a separate to address this change. There are a few places beyond this PR.
| ~FreeOrbital(); | ||
|
|
||
| std::string getClassName() const override { return "FreeOrbital"; } |
There was a problem hiding this comment.
If this is just used for error reporting/debugging, which is what I see, its fine if we need runtime reflection I don't think we should do it via string matching.
There was a problem hiding this comment.
Agreed. If we need to make selection based on derived class types, we should do enum not strings. It is only intended for printing.
| sposet = std::move(rot_spo); | ||
| #endif | ||
| } | ||
|
|
||
| if (!spo_object_name.empty() && sposet->getName().empty()) | ||
| sposet->setName(spo_object_name); | ||
| //if (!spo_object_name.empty() && sposet->getName().empty()) |
There was a problem hiding this comment.
Please allow me to do it in my next PR.
|
Test this please |
|
Test this please |
Review can be done independent from #4158 but need its fix of tests before merging.
Proposed changes
The major change is in SPOSet.h
What type(s) of changes does this code introduce?
Does this introduce a breaking change?
What systems has this change been tested on?
epyc-server
Checklist