Skip to content

Conversation

@tquatmann
Copy link
Member

@tquatmann tquatmann commented Feb 9, 2026

Storm can now be linked with libarchive as an optional dependency

New features:

  • Developer: Conveniently read and write binary and textual files from and to an archive (without writing the intermediate file to disk first)
  • DRN: Control the precision of floats in DRN export using --io:digits option
  • DRN: Reading from and writing to compressed DRN files (use .drn.gz / drn.xz file extensions and/or the new --compression option) (requires libarchive)
  • DRN: add @value_type section in DRN files to determine the type of the parsed values.
  • DRN: support for parsing of DRN interval models from CLI (if they have a corresponding @value_type section)

Once this is merged, we need to adapt this page and add libarchive to the optional dependencies on the website.
Side note: I noted that the DRN page does not mention the @nr_choices section.

- Storm can now be linked with libarchive as an optional dependency
- Auxiliary functions to conveniently read and write binary and textual files from and to an archive
- allow to control the precision of floats in DRN export using --io:digits option
- allow reading from and writing to compressed DRN files (use .drn.gz / drn.xz file extensions and/or the new --compression option) (requires libarchive)
…arsed values.

- support for parsing of DRN interval models from CLI (if they have a @value_type section)
@tquatmann
Copy link
Member Author

tquatmann commented Feb 9, 2026

I marked this as a draft since I haven't tested the DRN @value_type logic yet.

# following https://stackoverflow.com/a/76916842
# Homebrew ships libarchive keg only, include dirs have to be set manually
execute_process(
COMMAND brew --prefix libarchive
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes that it has been installed with brew? Can/Should we maybe first check whether it is somehow already present in the system otherwise via find_package?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I now see that it is always searched fore and that we are only setting a prefix variable, which is implicitly used by the findpackage below? I am not sure using PREFIX is better than passing this as a hint to the find package?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seconded. There are people on mac not using homebrew ;).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the execute_process is used to get a hint to catch the common case where brew is used. It fails quietly if brew is not installed.
I added some comments to clarify this.

Any advice on how to avoid this brew hack? And/or adding a hint to the find_package?
Setting LibArchive_INCLUDE_DIR seems to work most of the time, but I'd rather set the hint.

Relevant:

> brew info libarchive
[...]
libarchive is keg-only, which means it was not symlinked into /opt/homebrew,
because macOS already provides this software and installing another version in
parallel can cause all kinds of trouble.

If you need to have libarchive first in your PATH, run:
  echo 'export PATH="/opt/homebrew/opt/libarchive/bin:$PATH"' >> ~/.zshrc

For compilers to find libarchive you may need to set:
  export LDFLAGS="-L/opt/homebrew/opt/libarchive/lib"
  export CPPFLAGS="-I/opt/homebrew/opt/libarchive/include"

For pkgconf to find libarchive you may need to set:
  export PKG_CONFIG_PATH="/opt/homebrew/opt/libarchive/lib/pkgconfig"
[...]

The version shipped with macOS apparently does not have any include files.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use the HINTS for find_package instead of setting the include directory? Alternatively, you could try setting LibArchive_ROOT.
Last idea could be to write a FindLibArchive.cmake which contains the homebrew workaround. This might then also work if we install Storm.

set(LibArchive_INCLUDE_DIR "${LIBARCHIVE_PREFIX}/include")
else()
message(STATUS "Storm - LibArchive not found in ${LIBARCHIVE_PREFIX}/include.")
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have an else case here that also reports that libarchive is not found?

Copy link
Member Author

@tquatmann tquatmann Feb 9, 2026

Choose a reason for hiding this comment

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

That output was just to see if the brew hint was successful. I removed it because it is confusing.
There is always some kind of output after find_package

endif()
else()
SET(STORM_HAVE_LIBARCHIVE OFF)
message (WARNING "Storm - Not linking with LibArchive. Loading and exporting compressed files such will not be supported.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a status or a warning?

Copy link
Member Author

Choose a reason for hiding this comment

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

STATUS seems to be more in line with other dependencies.

@volkm
Copy link
Contributor

volkm commented Feb 9, 2026

Side note: I noted that the DRN page does not mention the @no_choices section.

I think it should be @nr_choices which is mentioned.

else()
SET(STORM_HAVE_LIBARCHIVE OFF)
message (WARNING "Storm - Not linking with LibArchive. Loading and exporting compressed files such will not be supported.")
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning message above may explicitly mention UMB (even uncompressed, as it is packed!)

)
endif()

if(@STORM_HAVE_LIBARCHIVE@) # STORM_HAVE_LIBARCHIVE
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this code not need the brew workaround?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ohh, it might actually need it.

*/
Iterator& operator++();

ArchiveReadEntry operator*() const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Return a reference to?

Copy link
Member Author

Choose a reason for hiding this comment

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

ArchiveReadEntry essentially wraps around two (non-owning) pointers.
Returning a reference here has no real benefit and we would need to have an entity that "owns" ArchiveReadEntries.

@tquatmann
Copy link
Member Author

Side note: I noted that the DRN page does not mention the @no_choices section.

I think it should be @nr_choices which is mentioned.

Ahh, true. I did not read that paragraph carefully enough. Sorry :)

@tquatmann
Copy link
Member Author

For future reference, I used the following script to test DRN import/export from CLI a little

mkdir -p tmp
rm tmp/*

set -e
STORM=./build/bin/storm
EXAMPLES=./resources/examples/testfiles

# brp (DTMC)

$STORM --prism $EXAMPLES/dtmc/brp-16-2.pm --exportbuild tmp/brp-double.drn
grep double tmp/brp-double.drn
$STORM -drn tmp/brp-double.drn
$STORM -drn tmp/brp-double.drn --exact
$STORM -drn tmp/brp-double.drn --parametric

$STORM --prism $EXAMPLES/dtmc/brp-16-2.pm --exportbuild tmp/brp-double.drn.xz
$STORM -drn tmp/brp-double.drn.xz

$STORM --prism $EXAMPLES/dtmc/brp-16-2.pm --exportbuild tmp/brp-double.drn.gz
$STORM -drn tmp/brp-double.drn.gz

$STORM --prism $EXAMPLES/dtmc/brp-16-2.pm --exportbuild tmp/brp-exact.drn --exact
grep rational tmp/brp-exact.drn
$STORM -drn tmp/brp-exact.drn
$STORM -drn tmp/brp-exact.drn --exact
$STORM -drn tmp/brp-exact.drn --parametric

# polling (MA)

$STORM --prism $EXAMPLES/ma/polling.ma -const N=3,Q=3 --exportbuild tmp/pol-double.drn --buildchoicelab
grep double tmp/pol-double.drn
$STORM -drn tmp/pol-double.drn
$STORM -drn tmp/pol-double.drn --buildchoicelab
$STORM -drn tmp/pol-double.drn --exact
$STORM -drn tmp/pol-double.drn --parametric

# robot (IMDP)
$STORM --prism $EXAMPLES/imdp/robot.prism -const delta=0.5 --exportbuild tmp/robot-double.drn
grep "double-interval" tmp/robot-double.drn
$STORM -drn tmp/robot-double.drn

# nand (PDTMC)
$STORM --prism $EXAMPLES/pdtmc/nand-5-2.pm --exportbuild tmp/nand-par.drn --parametric
grep "parametric" tmp/nand-par.drn
$STORM -drn tmp/nand-par.drn --parametric

@tquatmann tquatmann marked this pull request as ready for review February 10, 2026 13:00
Copy link
Contributor

@volkm volkm 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 in general. Thanks for all the work.

I have mostly minor comments which should hopefully be fast to modify.

enum class CompressionMode { Default, None, Gzip, Xz };

/*!
* @return The expression mode whose string representation matches the given input
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @return The expression mode whose string representation matches the given input
* @return The compression mode whose string representation matches the given input

* Sets the bits in the given bucket to the given value.
* @param bucketIndex The bucket. Bucket index i refers to the 64 bits with index i*64 to i*64+63.
* @param value The values to set. E.g., 1ull sets bit i*64 to 1 and all other bits in the bucket to 0
* @note: bucketIndex must be below bucketCount(). Values for bitindices > size() are ignored
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @note: bucketIndex must be below bucketCount(). Values for bitindices > size() are ignored
* @note: bucketIndex must be below bucketCount(). Values for bitindices > bucketCount() are ignored

void setBucket(uint64_t bucketIndex, uint64_t value);

/*!
* Gets the bits in the given bucket to the given value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Gets the bits in the given bucket to the given value.
* Gets the bits in the given bucket.

* Gets the bits in the given bucket to the given value.
* @param bucketIndex The bucket. Bucket index i refers to the 64 bits with index i*64 to i*64+63.
* @return The values of the bits in the given bucket.
* @note: bucketIndex must be below bucketCount(). Values for bitindices > size() are always 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @note: bucketIndex must be below bucketCount(). Values for bitindices > size() are always 0
* @note: bucketIndex must be below bucketCount(). Values for bitindices > bucketCount() are always 0

*
* @return The number of buckets of the underlying storage.
*/
size_t bucketCount() const;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are moving the function, maybe move it after size()?
But not important.

Comment on lines +3 to +6
#include "storm/storage/BitVector.h"
#include "storm/utility/macros.h"

#include "storm/exceptions/NotSupportedException.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#include "storm/storage/BitVector.h"
#include "storm/utility/macros.h"
#include "storm/exceptions/NotSupportedException.h"
#include "storm/exceptions/NotSupportedException.h"
#include "storm/storage/BitVector.h"
#include "storm/utility/macros.h"

}
#else
ArchiveWriter::ArchiveWriter(std::filesystem::path const&, CompressionMode const) {
throw storm::exceptions::NotSupportedException() << "Writing archives is not supported. Storm is compiled without LibArchive.";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw storm::exceptions::NotSupportedException() << "Writing archives is not supported. Storm is compiled without LibArchive.";
throw storm::exceptions::MissingLibraryException() << "Writing archives is not supported. Storm is compiled without LibArchive.";

// Free the entry metadata after we finish writing
archive_entry_free(entry);
#else
throw storm::exceptions::NotSupportedException() << "Writing archives is not supported. Storm is compiled without LibArchive.";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw storm::exceptions::NotSupportedException() << "Writing archives is not supported. Storm is compiled without LibArchive.";
throw storm::exceptions::MissingLibraryException() << "Writing archives is not supported. Storm is compiled without LibArchive.";

unset(LibArchive_BREW_HINT)
endif()
find_package(LibArchive QUIET)
if(TARGET LibArchive::LibArchive)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use LibArchive_FOUND?

# following https://stackoverflow.com/a/76916842
# Homebrew ships libarchive keg only, include dirs have to be set manually
execute_process(
COMMAND brew --prefix libarchive
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use the HINTS for find_package instead of setting the include directory? Alternatively, you could try setting LibArchive_ROOT.
Last idea could be to write a FindLibArchive.cmake which contains the homebrew workaround. This might then also work if we install Storm.

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.

3 participants