mrc-ide / odin.dust

Compile odin to dust
https://mrc-ide.github.io/odin.dust
Other
3 stars 1 forks source link

Avoid compiler warnings with initial time varying #120

Closed richfitz closed 1 year ago

richfitz commented 1 year ago

This is basically impossible to test, because compiler warnings are really unportable.

At the moment though, this model produces a set of warnings, if you have things turned up noisy enough:

odin.dust::odin_dust({
  initial(x) <- step + 1
  update(x) <- x
})
   g++ -std=gnu++11 -I"/usr/share/R/include" -DNDEBUG  -I'/home/rich/lib/R/library/cpp11/include' -g -Wall -Wextra -pedantic -Wmaybe-uninitialized -Wno-unused-parameter -Wno-cast-function-type -Wno-missing-field-initializers -O2  -I/home/rich/lib/R/library/dust/include -DHAVE_INLINE -fopenmp -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-BNt0N4/r-base-4.2.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2  -c dust.cpp -o dust.o
   dust.cpp: In function ‘dust::pars_type<T> dust::dust_pars(cpp11::list) [with T = odin; cpp11::list = cpp11::r_vector<SEXPREC*>]’:
   dust.cpp:251:9: warning: typedef ‘using real_type = using real_type = double’ locally defined but not used [-Wunused-local-typedefs]
     251 |   using real_type = typename odin::real_type;
         |         ^~~~~~~~~
   dust.cpp: In function ‘cpp11::sexp dust::dust_info(const dust::pars_type<T>&) [with T = odin]’:
   dust.cpp:258:29: warning: variable ‘internal’ set but not used [-Wunused-but-set-variable]
     258 |   const odin::internal_type internal = pars.internal;
         |                             ^~~~~~~~
   In file included from /home/rich/lib/R/library/dust/include/dust/gpu/types.hpp:9,
                    from /home/rich/lib/R/library/dust/include/dust/gpu/launch_control.hpp:4,
                    from /home/rich/lib/R/library/dust/include/dust/gpu/device_resample.hpp:4,
                    from /home/rich/lib/R/library/dust/include/dust/gpu/dust_gpu.hpp:17,
                    from /home/rich/lib/R/library/dust/include/dust/r/dust.hpp:15,
                    from dust.cpp:1:
   /home/rich/lib/R/library/dust/include/dust/types.hpp: In function ‘dust::pars_type<T> dust::dust_pars(cpp11::list) [with T = odin; cpp11::list = cpp11::r_vector<SEXPREC*>]’:
   /home/rich/lib/R/library/dust/include/dust/types.hpp:22:40: warning: ‘internal.odin::internal_type::initial_x’ is used uninitialized in this function [-Wuninitialized]
      22 |     shared(shared_), internal(internal_) {
         |                                        ^
   dust.cpp:253:23: note: ‘internal.odin::internal_type::initial_x’ was declared here
     253 |   odin::internal_type internal;
         |                       ^~~~~~~~

With this PR it compiles cleanly with no warnings. The generated code is barely different:

diff --git a/prev/src/dust.cpp b/fixed/src/dust.cpp
index daae091..d0d4149 100644
--- a/prev/src/dust.cpp
+++ b/fixed/src/dust.cpp
@@ -248,14 +248,13 @@ inline cpp11::writable::integers integer_sequence(size_t from, size_t len) {
 namespace dust {
 template<>
 dust::pars_type<odin> dust_pars<odin>(cpp11::list user) {
-  using real_type = typename odin::real_type;
   auto shared = std::make_shared<odin::shared_type>();
   odin::internal_type internal;
+  internal.initial_x = 0;
   return dust::pars_type<odin>(shared, internal);
 }
 template <>
 cpp11::sexp dust_info<odin>(const dust::pars_type<odin>& pars) {
-  const odin::internal_type internal = pars.internal;
   const std::shared_ptr<const odin::shared_type> shared = pars.shared;
   cpp11::writable::strings nms({"x"});
   cpp11::writable::list dim(1);

This PR replaces #118, which was opened with the wrong branch name

codecov[bot] commented 1 year ago

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage :thumbsup:

Coverage data is based on head (f7549fb) compared to base (5bcc0da). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #120 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 7 7 Lines 1080 1086 +6 ========================================= + Hits 1080 1086 +6 ``` | [Impacted Files](https://codecov.io/gh/mrc-ide/odin.dust/pull/120?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mrc-ide) | Coverage Δ | | |---|---|---| | [R/generate\_dust.R](https://codecov.io/gh/mrc-ide/odin.dust/pull/120/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mrc-ide#diff-Ui9nZW5lcmF0ZV9kdXN0LlI=) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mrc-ide). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mrc-ide)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.