Skip to content

fix output#7317

Open
mohanchen wants to merge 5 commits intodeepmodeling:developfrom
mohanchen:20260507-fix_output
Open

fix output#7317
mohanchen wants to merge 5 commits intodeepmodeling:developfrom
mohanchen:20260507-fix_output

Conversation

@mohanchen
Copy link
Copy Markdown
Collaborator

fix output

@mohanchen mohanchen added Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0 labels May 7, 2026
@mohanchen mohanchen requested a review from dzzz2001 May 7, 2026 03:14
Comment on lines +99 to +102
const int nspin = kv.get_nspin();
const int nkstot = kv.get_nkstot();
const int nkpoints_real = (nspin > 0) ? (nkstot / nspin) : nkstot;
std::cout << std::setw(16) << nkpoints_real;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In K_Vectors::set_kup_and_kdw (source_cell/klist.cpp:651-664), nkstot is only doubled for nspin == 2; nspin == 4 falls through without modifying nkstot. So the new formula will display nkstot / 4 for noncollinear / SOC runs — a 4× underreport.
Suggest either: "const int real_nk = (kv.get_nspin() == 2) ? kv.get_nkstot() / 2 : kv.get_nkstot();" or, better, expose an accessor on K_Vectors that returns the spin-independent k-point count and use it everywhere this distinction matters.

dm_gint.push_back(gint_info_->get_hr<float>());

ModuleGint::transfer_dm_2d_to_gint(*gint_info_, dm_vec, dm_gint);
ModuleGint::transfer_dm2d(*gint_info_, dm_vec, dm_gint);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The old name documented the operation: take a 2D-block-cyclic DM and reshape it into the gint serial layout. transfer_dm2d reads as "transfer a 2D dm" which doesn't say where to. Since the rename ripples through 9 files plus timer/TITLE labels (which downstream profiling scripts may grep), the win has to clear a high bar. I'd push back on this unless there's a follow-up reason.

If you do want a shorter name, “dm_2d_to_gint” is a better landing spot: drop the verb transfer (the X_to_Y form already conveys "convert/reshape" by convention) but keep both the source layout (2d) and the destination (gint). It goes from 22 → 13 chars without losing information, and the timer/TITLE labels can migrate with a single grep.

Comment on lines +31 to +32
GlobalV::ofs_running << std::endl;
ModuleBase::GlobalFunc::OUT(GlobalV::ofs_running, "The readin total magnetization", para.inp.nupdown);
Copy link
Copy Markdown
Collaborator

@dzzz2001 dzzz2001 May 7, 2026

Choose a reason for hiding this comment

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

The first added line is indented with a tab; the second with spaces. The surrounding file uses spaces — please normalize both to 12 spaces (matching the surrounding code).

Comment on lines -346 to 347
GlobalV::ofs_running <<"\n Warning_Memory_Consuming allocated: "
GlobalV::ofs_running <<"\n *** Memory Allocation Warning *** "
<<" "<<name[find]<<" "<<consume[find]<<" MB" << std::endl;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A pre-existing bug: print(find) is called from both record and record_gpu and unconditionally prints consume[find] — never consume_gpu[find]. A GPU allocation that crosses the new 20 MB line will print whatever happens to be in the CPU slot (often 0). Worth fixing while you're here.

Recommended fix: pass the data, drop the index
Decouple print from the global table layout entirely. It only needs a name and a size:

// memory.h
static void print(const std::string& name, double size_mb);

// memory.cpp
void Memory::print(const std::string& mem_name, double size_mb)
{
    GlobalV::ofs_running << "\n *** Memory Allocation Warning *** "
        << " " << mem_name << " " << size_mb << " MB" << std::endl;
}

Then at every call site, replace print(find); with the explicit form:

// CPU path
if (consume[find] > memory_warning_threshold_mb) {
    print(name[find], consume[find]);
}

// GPU path
if (consume_gpu[find] > memory_warning_threshold_mb) {
    print(name_gpu[find], consume_gpu[find]);
}

This is the cleanest fix — print no longer has a hidden coupling to "which table am I being called from", and it becomes trivially testable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants