Damaris plugin by Yushan-Wang · Pull Request #693 · pdidev/pdi

Yushan-Wang · 2026-05-06T13:55:52Z

List of things to check before making a PR

Before merging your code, please check the following:

…se or on other configs options.

…ide other plugins (trace, mpi, Decl'HDF5, etc.)

…s in the YML file!

…ch element requiring any metadata value uses depends_on YAML attribute to ensure an update of its attributes value to Damaris lib once the metadata are exposed (using Damaris Parameters in the background)!

…ding auto initialize without on_init event, and adding some cmake instruction in enforce finding of Damaris when Damaris_ROOT is provided.

The condition to do that is to always have this sequence at the end of the simulation: PDI_finalize(); MPI_Finalize(); Or provide in the yml conf the code following PDI_finalize(); Code like the following could not be handled: PDI_finalize(); PC_tree_destroy(&conf); free(cur); free(next); MPI_Finalize();

This reverts commit db29c2c.

Fix-local-install-1

)

* Hide is_client from user, and ending damaris server in background: - The condition to do that is to always have this sequence at the end of the simulation: PDI_finalize(); MPI_Finalize(); - Or provide in the yml conf the code following PDI_finalize(); - Code like the following could not be handled for the moment. But in the future by providing the instructions to execute...: PDI_finalize(); PC_tree_destroy(&conf); free(cur); free(next); MPI_Finalize(); * update copyright * Removing on_init and on_finalize

* Example with only Damaris API (resolves #10)

jmorice91 · 2026-08-26T14:05:38Z

Some known issues of this implementation are defined in https://github.com/jmorice91/pdi/issues.
(The issue name become with [Damaris plugin])
These developments will be done in other steps.

jmorice91

A first review.

jmorice91 · 2026-09-01T13:38:32Z

This file is never used. Perhaps, we should remove for a first version?

@endamlabin : What is the objective of this file?

jmorice91 · 2026-09-01T13:47:43Z

+		ctx.logger().info("Plugin loaded successfully");
+	}
+
+	void data(const std::string& name, PDI::Ref ref)


missing description.
As the name of function is to general, we should change the name.

jmorice91 · 2026-09-01T13:48:07Z

+		}
+	}
+
+	void event(const std::string& event_name)


missing description.
As the name of function is to general, we should change the name.

jmorice91 · 2026-09-01T13:48:29Z

+		}
+	}
+
+	void ensure_damaris_is_initialized(const std::string& event_name)


missing description

jmorice91 · 2026-09-01T13:50:28Z

+		}
+	}
+
+	void damaris_init()


missing description

jmorice91 · 2026-09-01T14:34:46Z

+				// context().logger().error("The Damaris need write access on the data (`{}')", name);
+				throw PDI::System_error{"The Damaris need write access on the data `{}' ", name};
+			}
+		} else if (m_config.is_parameter_to_update(name)) {


Perhaps, add a comment on this condition?

jmorice91 · 2026-09-01T14:37:36Z

+				//MayBe a PDI_multi_expose is under traitement
+				multi_expose_transaction_dataname.emplace_back(name);


Can you give us a use case of these lines with an example?

jmorice91 · 2026-09-01T14:39:49Z

+				multi_expose_transaction_dataname.emplace_back(name);
+			}
+		} else { //Handle other situations...
+			multi_expose_transaction_dataname.emplace_back(name);


Can you give us a use case of these lines with an example?

jmorice91 · 2026-09-01T14:40:42Z

+			}
+		} else { //Handle other situations...
+			multi_expose_transaction_dataname.emplace_back(name);
+			//multi_expose_transaction_dataref.emplace_back(ref);


Please, remove this line.

jmorice91 · 2026-09-01T14:43:41Z

+				if (ds_write_info.when.to_long(context())) {
+					context().logger().debug("data `{}' will be written when = '{}'", name, ds_write_info.when.to_long(context()));
+
+					int32_t block = ds_write_info.block.to_long(context());


What is the block? What is the interest for a user?

jmorice91

General comment:
There is not test and no example with custom event name defined in specification tree.

jmorice91 · 2026-09-02T07:22:12Z

+				std::string finalize_event_name = m_event_handler.get_event_name(Event_type::DAMARIS_FINALIZE);
+				m_event_handler.damaris_api_call_event(context(), m_damaris, finalize_event_name, multi_expose_transaction_dataname);
+			}
+		} else if (m_event_handler.is_damaris_api_call_event(event_name)) {


Add a comment like the if before.

Suggested change

} else if (m_event_handler.is_damaris_api_call_event(event_name)) {

}

// use of defined names in damaris plugin to call damaris api

else if (m_event_handler.is_damaris_api_call_event(event_name)) {

jmorice91 · 2026-09-02T07:43:14Z

+};
+
+/** These default event names are for internal use. If a configured name is given, these ones will be surcharged */
+const std::unordered_map<Event_type, std::string> event_names


This variables depends on the naming of the event in the core of damaris plugin as he used in other files.
suggestion: damaris_event_names

jmorice91 · 2026-09-02T08:01:38Z

+		if (!m_damaris) {
+			if (m_config.events().find(event_name) != m_config.events().end()) {
+				//Means the first action received by the plugin wasn't fir initialization...
+				if (!m_config.init_on_event().empty() && event_name != m_config.init_on_event()) {


if the event name for init_on_event="", what is arrived?
(need to be checked)

Do you mean that in your code
m_config.init_on_event().empty() is equivalent to no init_on_event defined in the specification tree of damaris ?

Perhaps, we can add a boolean in the code to know if no init_on_event defined in the specification tree of damaris ?

jmorice91 · 2026-09-02T08:17:10Z

+		if (m_config.finalize_on_event().empty() && m_damaris) {
+			context().logger().debug("Calling  DAMARIS_FINALIZE in ~damaris_plugin()");
+			std::string finalize_event_name = m_event_handler.get_event_name(Event_type::DAMARIS_FINALIZE);
+			m_event_handler.damaris_api_call_event(context(), m_damaris, finalize_event_name, multi_expose_transaction_dataname);
+		}


Perhaps, we should add the "m_config.finalize_on_event() is not empty and m_damaris" in the case of the user doesn't call event to finalize in its code?

Question:
If the user doesn't call "DAMARIS_STOP" before "DAMARIS_FINALIZE", what damaris do when we call "damaris_finalize"?

jmorice91 · 2026-09-02T08:34:09Z

+
+struct Architecture_type {
+	int domain;
+	placement arch_placement;


placement is never used? Perhaps, we can remove for a first version.

Question: What is the placement for damaris?

jmorice91

some other comments:

jmorice91 · 2026-09-02T08:50:48Z

+
+/** These default event names are for internal use. If a configured name is given, these ones will be surcharged */
+const std::unordered_map<Event_type, std::string> event_names
+	= {{Event_type::DAMARIS_INITIALIZE, "initialize"},


The event name doesn't depend on damaris.

Suggested change

= {{Event_type::DAMARIS_INITIALIZE, "initialize"},

= {{Event_type::DAMARIS_INITIALIZE, "damaris_initialize"},

jmorice91 · 2026-09-02T08:51:13Z

+       {Event_type::DAMARIS_SIGNAL, "damaris_signal"},
+       {Event_type::DAMARIS_BIND, "damaris_bind"},
+       {Event_type::DAMARIS_STOP, "damaris_stop"},
+       {Event_type::DAMARIS_FINALIZE, "finalize"}};


The event name doesn't depend on damaris.

Suggested change

{Event_type::DAMARIS_FINALIZE, "finalize"}};

{Event_type::DAMARIS_FINALIZE, "damaris_finalize"}};

…nt handling

…ous code in Damaris plugin - Fix uninitialized pointer dereferences in DAMARIS_PARAMETER_GET/SET and DAMARIS_SET_BLOCK_POSITION/WRITE_BLOCK - Fix damaris_pdi_write_block returning bool instead of int, which collapsed distinct Damaris error codes down to 0/1 - Remove dead code: shadowed Damaris_cfg architecture members, unused reset_parameter_depends_on(vector)/reset_all_parameters_depends_on(), unnecessary static local - Use const auto& instead of by-value copies in get_updatable_parameters - Resolve and clean up all remaining "// Jacques:" review comments in damaris_cfg.cxx, merging them into proper doxygen where applicable - Add doxygen/explanatory comments across damaris_cfg.h/.cxx, damaris_api_call_handler.h/.cxx and damaris_wrapper.cxx for non-obvious variables and code paths

Revert the erroneous copyright changes to the other 5 files, and correctly extend damaris_api_call_handler.h's end years to 2026 without altering its start years (2015 for CEA, 2024 for Inria).

jmorice91 · 2026-09-04T10:34:05Z

+ * Copyright (C) 2015-2019 Commissariat a l'energie atomique et aux energies alternatives (CEA)
+ * Copyright (C) 2021 Institute of Bioorganic Chemistry Polish Academy of Science (PSNC)
+ * All rights reserved.


Suggested change

* Copyright (C) 2015-2019 Commissariat a l'energie atomique et aux energies alternatives (CEA)

* Copyright (C) 2021 Institute of Bioorganic Chemistry Polish Academy of Science (PSNC)

* All rights reserved.

* Copyright (C) 2026 Commissariat a l'energie atomique et aux energies alternatives (CEA)

* All rights reserved.

jmorice91 · 2026-09-04T10:40:02Z

+ * Copyright (C) 2015-2019 Commissariat a l'energie atomique et aux energies alternatives (CEA)
+ * Copyright (C) 2021 Institute of Bioorganic Chemistry Polish Academy of Science (PSNC)


The copyright need to be change.

jmorice91

The copyright is needed to be checked in all the files.

jmorice91 · 2026-09-04T10:46:11Z

+# Copyright (C) 2015-2024 Commissariat a l'energie atomique et aux energies alternatives (CEA)
+# Copyright (C) 2024-2026 Institut national de recherche en informatique et en automatique (Inria)


The copyright need to be updated.

Suggested change

# Copyright (C) 2015-2024 Commissariat a l'energie atomique et aux energies alternatives (CEA)

# Copyright (C) 2024-2026 Institut national de recherche en informatique et en automatique (Inria)

# Copyright (C) 2026 Commissariat a l'energie atomique et aux energies alternatives (CEA)

# Copyright (C) 2024-2026 Institut national de recherche en informatique et en automatique (Inria)

* Adding tests for multidimensional array with ghosts layer

@Yushan-Wang

* Adding a test with when condition * Apply suggestion from @Yushan-Wang

- #9: document what `block` means (local sub-domain index, relevant when architecture/domains > 1) in the README, Dataset_Write_Info, and at its read site in damaris.cxx - #12: confirm init_on_event().empty() is the intended "not configured" check, per direct confirmation - #13: fix DAMARIS_FINALIZE's auto-stop fallback to key off whether DAMARIS_STOP has actually run (new m_stopped flag) rather than whether stop_on_event was configured, closing the gap where an explicitly configured stop_on_event that never fires would let finalize run without ever stopping dedicated server ranks (traced via ~/damaris source: no server-side check ties finalize to a prior stop)

jmorice91 · 2026-09-08T11:13:39Z

 * `write`: list of data that will be write on the disk by damaris. Each data is composed with
  * `dataset`: The dataset in which the data will be written.
  * `position`: The starting position of the data (for each client process) with repect to the dataset.
+  * `block`(integer, default: 0): Which local sub-domain of this client process the data belongs to, when `architecture/domains` is greater than 1 (i.e. a single client manages several sub-domains/patches itself). Must be between `0` and `domains - 1`. Can be left at its default when `domains = 1`.


Where is the test for this option?
It would be nice to have a discuss about this point.

remove domains and block for the 1st release

Yushan-Wang · 2026-09-16T09:00:21Z

+include(FindPackageHandleStandardArgs)
+include(DamarisPluginUtils)
+
+set(Damaris_BASE_DIR /usr/local/lib/damaris /usr/lib/damaris /opt/damaris)


is this still needed?

Yushan-Wang · 2026-09-16T09:01:14Z

+include(DamarisPluginUtils)
+
+set(Damaris_BASE_DIR /usr/local/lib/damaris /usr/lib/damaris /opt/damaris)
+set(Damaris_VERSIONS 1.12.0)


shouldn't this version number be retrieved from Damaris lib?

Yushan-Wang · 2026-09-16T09:57:58Z

+            global: ['$psize[0]*($dsize[0]-2)', '$psize[1]*($dsize[1]-2)']
+            dimensions: [ '$dsize[0]', '$dsize[1]' ] # process dim, with ghosts/boundaries
+            ghosts: '1:1,1:1'


Suggested change

global: ['$psize[0]*($dsize[0]-2)', '$psize[1]*($dsize[1]-2)']

dimensions: [ '$dsize[0]', '$dsize[1]' ] # process dim, with ghosts/boundaries

ghosts: '1:1,1:1'

# some where in the config yaml we have NG = 2

global: ['$psize[0]*($dsize[0]-$NG*2)', '$psize[1]*($dsize[1]-$NG*2)']

dimensions: [ '$dsize[0]', '$dsize[1]' ] # process dim, with ghosts/boundaries

ghosts: '$NG:$NG,$NG:$NG'

jmorice91 · 2026-09-22T11:54:22Z

+		} else if (key == "init_on_event" || key == "on_init") {
+			m_init_on_event = PDI::to_string(value);
+			load_event(m_events, ctx, m_init_on_event, Event_type::DAMARIS_INITIALIZE);
+		} else if (key == "finalize_on_event" || key == "on_finalize") {
+			m_finalize_on_event = PDI::to_string(value);
+			load_event(m_events, ctx, m_finalize_on_event, Event_type::DAMARIS_FINALIZE);


It is would be better to use only one key to define this event.

jmorice91

some comment

jmorice91 · 2026-09-22T11:55:55Z

+					store.store_opt_FileMode_ = PDI::to_string(value);
+				} else if (key == "files_path") {
+					store.store_opt_FilesPath_ = PDI::to_string(value);
+					// bool create_dir = creation_directory_c_only(ctx, store.store_opt_FilesPath_);


We can remove these lines

jmorice91 · 2026-09-22T11:56:24Z

+/// @return true if the directory is created or exists
+/// NOTE: currently unused (kept as a pre-C++17 <filesystem> fallback) - creation_directory_cpp()
+/// below is the one actually called from parse_storages_tree().
+bool creation_directory_c_only(PDI::Context& ctx, const std::string& dir_name)


Never used. we can remove.

jmorice91 · 2026-09-22T11:59:09Z

+void retrive_nested_groups(std::string& dataset_elt_full_name, char delimiter, std::string nested_groups_names[], unsigned& index);
+template <typename DS_TYPE>
+void insert_dataset_elts_to_group(DS_TYPE varxml, std::string nested_groups_names[], unsigned index);
+//void insert_dataset_elts_to_group(damaris::model::DamarisVarXML varxml, std::string nested_groups_names[], unsigned index);


we can remove.

jmorice91 · 2026-09-22T12:00:06Z

+			}
+		}
+
+		else if (key == "start" || key == "get_is_client" || key == "is_client_get")


keep only one.

endamlabin and others added 30 commits January 16, 2025 12:40

Damaris plugin

1ba0a79

Removing specific event calls, and adding event handling on data expo…

857f16d

…se or on other configs options.

Debugging the mpi communicator issue; The Damaris plugin works alongs…

89787e4

…ide other plugins (trace, mpi, Decl'HDF5, etc.)

share some notes/suggestions

296d5ff

Update tests.yml to remove CI for draft PRs

6458d2b

remove CI for draft PRs

a71c012

Removing specific names and relying on configured event and data name…

cb01081

…s in the YML file!

Removing damaris parameters from yaml and only using PDI metadata. Ea…

d02cbe5

…ch element requiring any metadata value uses depends_on YAML attribute to ensure an update of its attributes value to Damaris lib once the metadata are exposed (using Damaris Parameters in the background)!

Merge branch 'main' into damaris_plugin

93254f5

Delete .vscode directory

d7cd877

Add robustness + some bugs fixing

491dc4c

Add robustness + some bugs fixing

c2a3ca2

Check whether the HDF5 files_path exists, and create it if not!

9ea38fc

Resolve double finalize issue (thanks to Jacques Morice help :) ), ad…

9a1738e

…ding auto initialize without on_init event, and adding some cmake instruction in enforce finding of Damaris when Damaris_ROOT is provided.

Revert "Hide is_client from user:" (#16)

f2e80c7

This reverts commit db29c2c.

update dependencies

dfeb956

remove bits/stdc++ header

a5d7c50

fix POD error with va_arg

8752bb7

revert change to CMake

2f5d3c1

update copyright

6c78645

fix merge conflit

a787c17

update author

1688182

fix helper message for example

db06cf9

fix merge conflit

abbf392

fix merge conflict

0e72d63

Merge pull request #13 from jmorice91/fix-local-install-1

c24ea4d

Fix-local-install-1

Ensure each PDI_access has an associated PDI_release (resolves #20) (#23

c45a969

)

Example damaris only (#19)

96f0930

* Example with only Damaris API (resolves #10)

Yushan-Wang requested a review from a team August 5, 2026 16:51

Yushan-Wang requested review from benoitmartin88 and jbigot as code owners August 5, 2026 16:51

jmorice91 added 3 commits August 6, 2026 17:07

fix indent and using

4c31fe3

Fix size_of, gtest_add_tests

e5b6ffa

fix indent

346a98c

jmorice91 reviewed Sep 1, 2026

View reviewed changes

jmorice91 reviewed Sep 2, 2026

View reviewed changes

endamlabin added 6 commits September 3, 2026 10:37

Remove unused damaris_async_gather.h

ba5111e

Remove dead code: unused multi_expose_transaction_dataref and placeme…

17f2496

…nt handling

Rename overly generic identifiers with a damaris_ prefix

70b303f

Fix previous copyright update: only damaris_api_call_handler.h needed it

f9610fc

Revert the erroneous copyright changes to the other 5 files, and correctly extend damaris_api_call_handler.h's end years to 2026 without altering its start years (2015 for CEA, 2024 for Inria).

jmorice91 reviewed Sep 4, 2026

View reviewed changes

jmorice91 and others added 4 commits September 6, 2026 18:26

adding tests with multidimensional data (#70)

264e5f3

* Adding tests for multidimensional array with ghosts layer

adding a test with when condition (#69)

450c450

* Adding a test with when condition * Apply suggestion from @Yushan-Wang

Fix some error

d05f636

jmorice91 reviewed Sep 8, 2026

View reviewed changes

Yushan-Wang commented Sep 16, 2026

View reviewed changes

jmorice91 reviewed Sep 22, 2026

View reviewed changes

		//MayBe a PDI_multi_expose is under traitement
		multi_expose_transaction_dataname.emplace_back(name);

	= {{Event_type::DAMARIS_INITIALIZE, "initialize"},
	= {{Event_type::DAMARIS_INITIALIZE, "damaris_initialize"},

	{Event_type::DAMARIS_FINALIZE, "finalize"}};
	{Event_type::DAMARIS_FINALIZE, "damaris_finalize"}};

		* Copyright (C) 2015-2019 Commissariat a l'energie atomique et aux energies alternatives (CEA)
		* Copyright (C) 2021 Institute of Bioorganic Chemistry Polish Academy of Science (PSNC)

		# Copyright (C) 2015-2024 Commissariat a l'energie atomique et aux energies alternatives (CEA)
		# Copyright (C) 2024-2026 Institut national de recherche en informatique et en automatique (Inria)

Conversation

Yushan-Wang commented May 6, 2026

List of things to check before making a PR

Uh oh!

jmorice91 commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmorice91 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmorice91 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmorice91 Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmorice91 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmorice91 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmorice91 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmorice91 commented Aug 26, 2026 •

edited

Loading

jmorice91 Sep 2, 2026 •

edited

Loading