algbio / ggcat

Compacted and colored de Bruijn graph construction and querying
MIT License
72 stars 10 forks source link

Using nightly Rust is problematic #27

Closed jnalanko closed 7 months ago

jnalanko commented 1 year ago

The build was again broken on latest nightly. It seems to be fixed now, but to prevent future breakage, I think it would be better if GGCAT used a fixed Rust version that is not the nightly version.

rob-p commented 1 year ago

I was talking with @jermp about this. Might I ask what nightly features are required in GGCAT? In general, nightly is highly unstable (some features on nightly might never make it into stable and, in fact, might be removed completely). Depending on beta or stable would be much more robust if there are only features being used that could be replaced by some macros or crates that work on stable.

Guilucand commented 1 year ago

Hi, the nightly features I currently use on GGCAT are these:

#![feature(const_type_id)]
#![feature(drain_filter)]
#![feature(int_roundings)]
#![feature(let_chains)]
#![feature(new_uninit)]
#![feature(panic_info_message)]
#![feature(slice_group_by)]
#![feature(slice_partition_dedup)]
#![feature(thread_local)]

#![feature(impl_trait_in_assoc_type)]

I think almost all of them can be removed by implementing the features in a separate library, But the impl_trait_in_assoc_type feature is very handy and cannot be easily replaced by something else, since simple alternatives usually add an extra layer of indirection that could affect the performance.

In the next days I'll try to remove as much unstable features as possible, and see if I can also find a workaround for the last one.

jermp commented 1 year ago

Hi, Actually I tried to remove the

![feature(impl_trait_in_assoc_type)]

and everything went smooth with the stable channel release.

leoisl commented 1 year ago

Hello, thanks a lot for this tool, I'd like to experiment querying plasmids and AMR genes against the 661k bacterial dataset with it. I am having the same issue but I do not know rust, sorry. I get this error when trying to install in my cluster:

   Compiling ggcat_kmers_transform v0.1.0 (~/ggcat/crates/kmers_transform)
error[E0635]: unknown feature `drain_filter`
 --> crates/kmers_transform/src/lib.rs:2:12
  |
2 | #![feature(drain_filter)]
  |            ^^^^^^^^^^^^

   Compiling ggcat_assembler_minimizer_bucketing v0.1.0 (~/ggcat/crates/assembler_minimizer_bucketing)
For more information about this error, try `rustc --explain E0635`.
error: could not compile `ggcat_kmers_transform` (lib) due to previous error
warning: build failed, waiting for other jobs to finish...
error: failed to compile `ggcat_cmdline v0.1.0 (~/ggcat/crates/cmdline)`, intermediate artifacts can be found at `~/ggcat/target`.

... should I keep trying from time to time until there is a nightly version that does not break?

rob-p commented 1 year ago

What is your current rustc version?

Guilucand commented 1 year ago

Hi, I worked on the removal of the nightly features, and now only three of them remain:

#![feature(impl_trait_in_assoc_type)]
#![feature(thread_local)]
#![feature(slice_partition_dedup)]

The updated code (that should compile on a wider range of nightly rust versions) is available in the dev branch

sebschmi commented 1 year ago

Nowadays, you can use the rust-toolchain.toml to pin to the nightly version of a certain date.

Modified example from the docs:

[toolchain]
channel = "nightly-2023-09-07"
Guilucand commented 8 months ago

Good news, with the stabilization of the feature return_position_impl_trait_in_trait in Rust 1.75 GGCAT now uses no unstable features.

The final transition to stable Rust is still blocked by a small change that needs to be made to the matchtigs library, for which I just made a pull request.

rob-p commented 7 months ago

RPITIT FTW! Yay acronyms.

jermp commented 7 months ago

Great! Keep us posted :)

Guilucand commented 7 months ago

I managed to support stable even without the matchtigs library fix, so now GGCAT works on stable Rust >= 1.75 :)