Skip to content

Resolve "implement Hamiltonian variables check"#564

Open
ALF-Import-Bot wants to merge 19 commits intomasterfrom
316-implement-hamiltonian-variables-check
Open

Resolve "implement Hamiltonian variables check"#564
ALF-Import-Bot wants to merge 19 commits intomasterfrom
316-implement-hamiltonian-variables-check

Conversation

@ALF-Import-Bot
Copy link

In GitLab by @lstocker on Jul 23, 2025, 08:53 UTC:

Closes #316
Closes #276

Assignees: lstocker

Reviewers: Jonas_schwab

Approved by: Jonas_schwab

Migrated from GitLab: https://git.physik.uni-wuerzburg.de/ALF/ALF/-/merge_requests/236

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Jul 23, 2025, 08:53 UTC:

requested review from @johanneshofmann87

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 4, 2025, 12:25 UTC:

added 3 commits

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 4, 2025, 13:24 UTC:

added 1 commit

  • 6c31326 - runtime variables on separatedfile

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 4, 2025, 13:33 UTC:

added 1 commit

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 4, 2025, 13:50 UTC:

I created a separate file QMC_runtime_var, which is used by main and can also be accessed by Hamiltonian_##NAME##_smod (or any other file) to read QMC variables.

As of now, if, for example, a Hamiltonian does not support the sequential update scheme, yet "Sequential": true is set (using pyalf notation), one should add the following lines to Hamiltonian_##NAME##_smod.F90:

Subroutine Ham_Set
    Use QMC_runtime_var

    ...

    if (Sequential) then
        Write(error_unit,*) 'Sequential update scheme not applicable to the Hamiltonian_##NAME##'
        CALL Terminate_on_error(ERROR_GENERIC, __FILE__, __LINE__)
    endif

    ...
end Subroutine Ham_Set

This is not the most elegant solution, as it raises an error only when calling .run(), but I do not think we can overcome this (as the filename suggest, we are dealing with runtime variables and, even if in this case one could stop the code at compile time already, this would imply a mix of the two types).

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 4, 2025, 14:00 UTC:

added 1 commit

  • dae2aca - Revert "rename MC_var -> QMC_var"

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 4, 2025, 14:08 UTC:

added 1 commit

  • 1b873a2 - rename MC_runtime_var -> QMC_runtime_var

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @Jonas_schwab on Aug 5, 2025, 08:59 UTC:

I would move the reading and MPI broadcast of the variables to the module. This will clean up main.F90 a bit.

For added cleanliness, one could even make most of the variables private and access them via getters, e.g. functions like get_sequential().

@ALF-Import-Bot
Copy link
Author

Discussion in GitLab:

In GitLab by @Jonas_schwab on Aug 5, 2025, 09:08 UTC:

Should we move all these variable in there? Giving the Hamiltonian access to so many variables opens avenues for breaking backwards compatibility and additional ways for user to shoot themselves in the foot. Especially things like Stab_nt and udvst are probably better kept for internal use.

There are also a few things that don't belong in such a big scope, like ierr and toggle. I suppose they're mainly there because of the early state of this draft.

In GitLab by @johanneshofmann87 on Aug 5, 2025, 09:28 UTC:

Yes, this is a very early stage of the draft.

@ALF-Import-Bot
Copy link
Author

In GitLab by @johanneshofmann87 on Aug 5, 2025, 09:26 UTC:

@Jonas_schwab and @lstocker ,
We can probably be a little bit more selective and focus on actual QMC setting variables. I wouldn't mind to make some variables private, but one of the ideas is that we can get access to them from within the model. In some special implementations, for example, sequential updates are no longer allowed and should be disabled. In the past, I was then just not allocating some Operator members or used other "hacks" to trigger segmentation faults. While this worked for me and I could recall what was going on, this is a pretty dirty workaround and not maintainable in the long run.
So we would have to provide "get" and "set" routines.
@lstocker : I'll mark some of the variables as comments in the source code changes section.



! Space for reading in Langevin & HMC parameters
Logical :: Langevin, HMC
Copy link
Author

Choose a reason for hiding this comment

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

In GitLab by @johanneshofmann87 on Aug 5, 2025, 09:37 UTC:

Primarily, these variables of this namelist (and potential dependencies) are settings in the parameter file and should be in this new module


NAMELIST /VAR_HAM_NAME/ ham_name

! General
Copy link
Author

Choose a reason for hiding this comment

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

In GitLab by @johanneshofmann87 on Aug 5, 2025, 09:37 UTC:

these general variables can likely remain in main and don't need to become "global" variables. The only variable that could be kept in this new module is Mc_step_weight, but I'm not sure yet.

Copy link
Author

Choose a reason for hiding this comment

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

In GitLab by @lstocker on Aug 6, 2025, 15:07 UTC:

changed this line in version 5 of the diff

Copy link
Author

Choose a reason for hiding this comment

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

In GitLab by @lstocker on Aug 6, 2025, 15:08 UTC:

See latest commit.

@ALF-Import-Bot
Copy link
Author

In GitLab by @johanneshofmann87 on Aug 5, 2025, 09:37 UTC:

left review comments

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 6, 2025, 15:07 UTC:

added 2 commits

  • 73ecab1 - reorder QMC_runtime_var_mod.mod alphabetically
  • 3395d20 - move runtime initialization and broadcast to QMC_runtime_var_mod

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 7, 2025, 14:21 UTC:

added 1 commit

  • 940d96f - fix mpi variable init and moving var to main

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 8, 2025, 09:07 UTC:

added 1 commit

  • 86ee3a0 - fix missing broadcasting of runtime variables

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 15, 2025, 14:03 UTC:

added 1 commit

  • a5d09b1 - move parts of warnings for schemes/variables incompatibility to QMC_runtime_var

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 18, 2025, 08:47 UTC:

added 1 commit

  • dedfb0f - fix non-initialized variable for tempering

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 20, 2025, 11:05 UTC:

added 1 commit

  • ccee745 - declare LOBS_ST, LOBS_EN as inout variables for related subroutine

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Aug 20, 2025, 11:34 UTC:

added 1 commit

  • d803a2d - remove redefinition of LOBS_EN, LOBS_ST

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @johanneshofmann87 on Sep 12, 2025, 14:03 UTC:

added 2 commits

  • 7c57897 - moving Nbins_eff back to main, putting tempering settings in new module
  • 070b85d - Merge branch '316-implement-hamiltonian-variables-check' of...

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @johanneshofmann87 on Sep 12, 2025, 14:05 UTC:

@lstocker I have slightly modified your version. Let me know what you think, otherwise I would be ready to start the integration process by asking Jonas for his input :)

@ALF-Import-Bot
Copy link
Author

In GitLab by @lstocker on Sep 12, 2025, 14:24 UTC:

Looks good to me, go ahead!

@ALF-Import-Bot
Copy link
Author

In GitLab by @johanneshofmann87 on Sep 12, 2025, 14:42 UTC:

marked this merge request as ready

@ALF-Import-Bot
Copy link
Author

In GitLab by @johanneshofmann87 on Sep 12, 2025, 14:42 UTC:

approved this merge request

@ALF-Import-Bot
Copy link
Author

In GitLab by @johanneshofmann87 on Sep 12, 2025, 14:43 UTC:

requested review from @johanneshofmann87 and removed approval

@ALF-Import-Bot
Copy link
Author

In GitLab by @johanneshofmann87 on Sep 12, 2025, 14:43 UTC:

requested review from @Jonas_schwab and removed review request for @johanneshofmann87

@ALF-Import-Bot
Copy link
Author

Discussion in GitLab:

In GitLab by @Jonas_schwab on Sep 23, 2025, 11:28 UTC:

Looks good to me. Though, I would also move the reading of VAR_QMC into the module (Along with ham_name, since otherwise this would make it more complicated again.). I can do that if you want.

In GitLab by @johanneshofmann87 on Sep 23, 2025, 11:32 UTC:

Sure, please feel free to go ahead. I was considering it at some point as well.

In GitLab by @Jonas_schwab on Sep 23, 2025, 12:45 UTC:

Done.

I've also moved Isize, Irank, Irank_g, Isize_g, color, key, igroup into the module. Though if I think about it now, it's not so clean. I'll probably revert that.

In GitLab by @lstocker on Sep 23, 2025, 13:11 UTC:

I initially did the same. But after discussing with Johannes, we concluded that keeping them in the main is the better option.

@ALF-Import-Bot
Copy link
Author

In GitLab by @Jonas_schwab on Sep 23, 2025, 12:31 UTC:

added 1 commit

  • 281ef69 - Moved reading of VAR_QMC and ham_name into QMC_runtime_var_mod

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @Jonas_schwab on Sep 23, 2025, 12:35 UTC:

added 1 commit

  • 24db0d3 - main.F90: Remove unused variables

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @Jonas_schwab on Sep 23, 2025, 13:26 UTC:

added 1 commit

  • 0ea3191 - Move MPI variables back to main.F90

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @Jonas_schwab on Sep 23, 2025, 14:42 UTC:

approved this merge request

@ALF-Import-Bot
Copy link
Author

In GitLab by @johanneshofmann87 on Sep 29, 2025, 12:52 UTC:

resolved all threads

@ALF-Import-Bot
Copy link
Author

In GitLab by @fassaad on Sep 29, 2025, 13:01 UTC:

This is of course much cleaner. Have we really checked all the environment variables?

@ALF-Import-Bot
Copy link
Author

In GitLab by @fassaad on Oct 28, 2025, 15:56 UTC:

Put a lock on the variables. You can only change the variable once. Setters and getters. Variables have to be private, and write routines to update them. These routines can issue warnings!

@ALF-Import-Bot
Copy link
Author

In GitLab by @Jonas_schwab on Oct 28, 2025, 18:33 UTC:

added 1 commit

  • db8a6242 - runtime_error_mod: Use setters and getters

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @Jonas_schwab on Oct 28, 2025, 18:41 UTC:

added 1 commit

  • 0344a07 - QMC_runtime_var: Use setters and getters

Compare with previous version

@ALF-Import-Bot
Copy link
Author

In GitLab by @Jonas_schwab on Oct 28, 2025, 18:42 UTC:

I took the liberty to add setters and getters (with a little help from ChatGPT).

We could now add something like a bool allow_override_var.

@ALF-Import-Bot
Copy link
Author

In GitLab by @Jonas_schwab on Oct 28, 2025, 18:45 UTC:

added 76 commits

  • 0344a07...a5e4c2f - 75 commits from branch master
  • d5b856b - Merge branch 'master' into 316-implement-hamiltonian-variables-check

Compare with previous version

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants