Skip to content

Add Correlator CLI and defaults#388

Merged
ElliottKasoar merged 23 commits intostfc:mainfrom
harveydevereux:correlator_cli
Mar 20, 2025
Merged

Add Correlator CLI and defaults#388
ElliottKasoar merged 23 commits intostfc:mainfrom
harveydevereux:correlator_cli

Conversation

@harveydevereux
Copy link
Collaborator

@harveydevereux harveydevereux commented Jan 21, 2025

  • Adds CLI arguments for requesting correlations defined in janus_core.processing.observables.
  • Adds default values for blocks=1, points=1, averaging=1 and update_frequency=1 injanus_core.processing.correlator.Correlation.
  • Add default for Velocity (all components, all atoms)
  • Fixes docstring in md.py.

Last thing to do is add text in the user_guide


Currently this is example functionality

janus md \
--ensemble nve --struct tests/data/NaCl.cif \
--steps 10 --correlation-kwargs \
"{'vaf1': {'a': 'velocity', 'points': 10}, 'vaf_x': {'a': 'velocity', 'update_frequency': 2, 'points': 10, 'a_kwargs': {'components': ['x']}, 'b_kwargs': '.'}}"

Syntax

  • If one of 'a' and 'b' is omitted it is duplicated.
  • The special "ditto" kwarg value "." indicates copying the kwargs.
  • 'blocks', 'points' etc have defaults which are not thought out yet.

it results in

cat NaCl-nve-T300.0-cor.dat
vaf1:
  lags: [0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0]
  value: [0.0005823628861742493, 0.0005829660336725057, 0.0005832862278350698, 0.0005833234400626741,
    0.0005830778925578795, 0.0005825500577781349, 0.0005817406577431897, 0.0005806506631988626,
    0.0005792812926375899, 0.0005776340111746314]
vaf_x:
  lags: [0.0, 1.0, 2.0, 3.0, 4.0, 5.0]
  value: [0.0005445815119304412, 0.0005449602181734653, 0.0005449448511073212, 0.0005445360209632227,
    0.0005437350402279519, 0.0005425439175493092]

@ElliottKasoar ElliottKasoar added the enhancement New/improved feature or request label Jan 22, 2025
@ElliottKasoar ElliottKasoar linked an issue Jan 22, 2025 that may be closed by this pull request
@harveydevereux harveydevereux changed the title Add Correlator CLI Add Correlator CLI and defaults Jan 24, 2025
@harveydevereux harveydevereux marked this pull request as ready for review February 14, 2025 15:19
Copy link
Member

@ElliottKasoar ElliottKasoar left a comment

Choose a reason for hiding this comment

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

Mostly minor comments/tidying up, but looks good so far, thanks!

Copy link
Member

@ElliottKasoar ElliottKasoar left a comment

Choose a reason for hiding this comment

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

A couple of places where I don't think the type hints are needed anymore, but generally looks good.

Could you add an example to the CLI docs (with a line or two describing roughly the composition, since it's not obvious)?

I think we still want a bit more testing on the different ways of specifying things too.

@ElliottKasoar ElliottKasoar requested a review from oerc0122 March 3, 2025 14:04
@alinelena
Copy link
Member

will be nice to have this in for training

Copy link
Member

@ElliottKasoar ElliottKasoar left a comment

Choose a reason for hiding this comment

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

In the summary, a couple of the outputs haven't been properly parsed by the tuple and dict conversion utils. Can you try to have a look? E.g.

  correlation_kwargs: !!python/object:janus_core.cli.types.TyperDict
    value:
      vaf:
        a: Velocity
        a_kwargs: &id001
          atoms_slice: !!python/tuple
          - 0
          - null
          - 2
        b_kwargs: *id001
        points: 100

(Paths should be converted to strings, and tuples to lists, with a couple of functions called in cli/utils/start_summary.)

Also worth rebasing again, to check the output file changes don't clash.

harveydevereux and others added 7 commits March 20, 2025 12:16
Co-authored-by: ElliottKasoar <45317199+ElliottKasoar@users.noreply.github.com>
Also copy Observable kwargs if not specified and copying observable
Co-authored-by: ElliottKasoar <45317199+ElliottKasoar@users.noreply.github.com>
Check for unexpected correlation_kwargs
@ElliottKasoar ElliottKasoar merged commit 44685f7 into stfc:main Mar 20, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New/improved feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support the online correlator in the cli

4 participants