Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have HDF5 write raise error if operator(s) requested #3951

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

rupertnash
Copy link
Contributor

Discussion in #3942 suggests that support for operators in "secondary" (for want of a better term) engines is not planned. My suggestion that ADIOS should let the user know this won't happen is added in this PR.

@pnorbert
Copy link
Contributor

pnorbert commented Dec 1, 2023

Thanks. I have to think about this. I am not sure if every user would want the code abort with an exception rather than quietly ignore the operator. On the other hand, as far as I know, you are the one and only user of the HDF5 engine.

@rupertnash
Copy link
Contributor Author

I would argue that silently not doing something that the docs explicitly saw will be done is worse that giving an error.

@rupertnash
Copy link
Contributor Author

Hopefully made clang format happy now!

@eisenhauer
Copy link
Member

I would argue that silently not doing something that the docs explicitly saw will be done is worse that giving an error.

We might think about a one-time rank-zero warning printout rather than an exception. (@anagainaru If we had a facility for this, we might also use it for warning about the overheads of reader-side derived variable computation.)

@pbartholomew08
Copy link

Could there be an option to make warnings into errors (similar to -Werror) in that case? Then if users absolutely don't want it silently ignored it will stop?

@anagainaru
Copy link
Contributor

Could there be an option to make warnings into errors (similar to -Werror) in that case? Then if users absolutely don't want it silently ignored it will stop?

Yes, I think this would be a good idea. I don't think we have the option now but we should look into this in the future. I am strongly in favor of giving at least some warning if you set a parameter that will be ignored.

@eisenhauer
Copy link
Member

Could there be an option to make warnings into errors (similar to -Werror) in that case? Then if users absolutely don't want it silently ignored it will stop?

Yes, I think this would be a good idea. I don't think we have the option now but we should look into this in the future. I am strongly in favor of giving at least some warning if you set a parameter that will be ignored.

Taking this literally, we have long thought it might be useful to warn if you set an engine parameter that is never consumed (because you might have just misspelled it). But our disaggregated method of looking at parameters makes this hard. This case of ignoring an operator is at least easier to detect. But norbert's got a point too...

@guj guj assigned guj and unassigned guj Dec 5, 2023
@guj guj self-requested a review December 5, 2023 19:31
Copy link
Contributor

@guj guj left a comment

Choose a reason for hiding this comment

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

Looks good.

@eisenhauer eisenhauer merged commit 8776d5d into ornladios:master Dec 12, 2023
35 checks passed
pnorbert added a commit to pnorbert/ADIOS2 that referenced this pull request Dec 12, 2023
* master:
  Have HDF5 write raise error if operator(s) requested (ornladios#3951)
  fix for ASAN issue related to JoinedDimArray handling in BP5 deserializer (ornladios#3963)
  New operator MDR, for refactoring floating point arrays using MGARD's new MDR extension. (ornladios#3826)
  restricted http transport from windows builds.
  XMLConfigTest: Add RemoveIO test
  adios2::core::ADIOS: Initialize new IO objects with config file
  removed unsused variable
  Update readme for heat transfer example with new location and build instructions
  Ignore tests with defects for now
  Adapt libfabric dataplane of SST to Cray CXI provider (ornladios#3672)
  ci: fix path to lsan suppressions, fix broken gh status post
  Use adios2_mode_readRandomAccess in matlab open to make it work for BP5 (ornladios#3956)
  Add Global Array Capabilities and Limitations
  Add Section for Anatomy of an ADIOS Program
  Enable Shell-Check for gh-actions scripts
  Enable Shell-Check for circle CI scripts
  Enable Shell-Check for tau contract scripts
  Enable Shell-Check for scorpio contract scripts
  Enable Shell-Check for lammps contract scripts
  Delete VTK code in examples
  Fix MATLAB bindings for MacOS (ornladios#3950)
  Set the compiler for the Kokkos DataMan example to what is used to build Kokkos
  Fix the HIP architecture CMAKE variable (ornladios#3931)
  perfstubs 2023-11-27 (845d0702) (ornladios#3944)
  Revert "Only rank 0 should print the initialization message in perfstub"
  Formatting
  Formatting
  Revision
  Added buffered data receive in the client side.
  A socket version of HTTP connector. Proxy server host is hardwired to "localhost" and port to 9999 Remote bpls: bpls -E bp4 -T "Library=HTTP" /remote_path/myVector_cpp.bp -d bpInts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants